On 7/7/21 2:24 PM, Thomas Lamprecht wrote:
On 07.07.21 13:23, Dominik Csapak wrote:
On 7/7/21 12:19 PM, Thomas Lamprecht wrote:
On 07.07.21 10:47, Dominik Csapak wrote:
diff --git a/www/manager6/ceph/Status.js b/www/manager6/ceph/Status.js
index e92c698b..52563605 100644
--- a/www/manager6/ceph/Status.js
+++ b/www/manager6/ceph/Status.js
@@ -321,14 +321,14 @@ Ext.define('PVE.node.CephStatus', {
       let unhealthy = degraded + unfound + misplaced;
       // update recovery
       if (pgmap.recovering_objects_per_sec !== undefined || unhealthy > 0) {
-        let toRecover = pgmap.misplaced_total || pgmap.unfound_total || 
pgmap.degraded_total || 0;
-        if (toRecover === 0) {
+        let totalRecovery = pgmap.misplaced_total || pgmap.unfound_total || 
pgmap.degraded_total || 0;

why change the variable name, `toRecover` was still OK? Or at least I do not see
any improvement in making it easier to understand with `totalRecovery` if byte 
vs.
objects where a issue of confusion why not addressing that by using 
`toRecoverObjects`
or the like
i read the code and thought 'toRecover' means objects that need recovery, but 
it is not. {misplaced,unfound,degraded}_total each contain
the total number of objects taking part in the recovery
(also the ones that are not unhealthy)

maybe 'totalRecoveryObjects' would make more sense ?

totalRecoveryObjects and toRecoverObjects are so similar that they do not really
convey the difference to me for the confusion you had for any other reader, for 
that
I'd rather add a short comment, those tend to be a bit more explicit for subtle 
stuff.

ok i'll leave it at 'toRecover' and add a comment what it is in my v2



Also, why not adding those metrics up? If, misplaced and unfound do not have any
overlap, IIRC, so would def. make sense for those - for degraded I'm not so sure
about overlap with the other two from top of my head though.

they contain all the same number
src/mon/PGMap.cc:{467,482,498} pool_sum.stats.sum.num_object_copies

ah yeah true, I remember now again. Do you also know where this is actually
set (computed).


no sadly, i tried to check, but i am not so deep into ceph code right now


_______________________________________________
pve-devel mailing list
[email protected]
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to