Ryan Harper has posted comments on this change.
Change subject: Move the lastCheck computation in getRepoStats
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(2 inline comments)
....................................................
File vdsm/storage/sp.py
Line 1355
Line 1356
Line 1357
Line 1358
Line 1359
I see in DomainStatus.py it already is doing a lastCheck = time(), we could
move the now() - lastCheck logic into DomainStatus.py and we wouldn't need to
change the code here.
Line 1370: res[sdUUID] = {
Line 1371: 'finish' : st.lastCheck,
Line 1372: 'result' : {
Line 1373: 'code' : code,
Line 1374: 'lastCheck' : '%.1f' % (now - st.lastCheck),
Do we? Isn't it possible for any of the function calls to take a long time
with large amounts of storage? What if the domain list is large?
> this loop can't take a long time and the "now" value is accurate enough
I understand wanting to avoid additional syscalls, but it's not clear to me how
we know this.
See above comment, we're already doing a syscall per st in any case, if we move
the now logic into the DomainStatus itself, then we get a value closer to when
we actually fetched the status and we avoid any additional syscalls, we're just
modifying how we present lastCheck.
Line 1375: 'delay' : str(st.readDelay),
Line 1376: 'valid' : (st.error is None)
Line 1377: },
Line 1378: 'disktotal' : disktotal,
--
To view, visit http://gerrit.ovirt.org/7993
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I58de0c517bc6940f21c7f6506854c9d9735e964b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Ryan Harper <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
