feat: extend update_tree_r_cache command with new features #1175
Conversation
|
I am sorry, the CI seems to be flaky since adding the new caching :( I still quite understand why though :/ |
Ah, I was sanity checking here, and of course things pass locally. Good to know it's not just me then :-) |
|
Regarding CI failures: I lack the context to know why we should feel comfortable ignoring what have the appearance of real test failures, which — if tests are accurate — would be significant. I don't think we should develop a habit of skipping or removing tests because they are failing. If we start down that road, it will be very hard to recover if a real problem goes undetected. Have we seen these failures elsewhere? We need to diagnose/debug this unless we have high confidence about what is going on and why it's not an immediate problem. (I can accept this might be true if someone presents the case to me.) |
|
After reading previous comments more closely: if the new caching made CI flaky, we should consider reverting the new caching until we can ascertain that this is a CI problem (and fix it). If there is any chance that the new caching is itself flaky and this is being exposed by the tests, we should not release it. If we want to claim this is not possible, we should consider what method we would use to detect intermittent incorrectness of the caching code. |
There was a problem hiding this comment.
This seems fine, and I don't think it will harm anything even if the answer to my questions is unsatisfying. I do have some questions about intended usage (after a somewhat cursory review). I'm approving in case there is short-term value in releasing this now, and on the assumption that this tool will be evolving regardless.
cryptonemo
force-pushed
the
extend-tree-r-tool
branch
2 times, most recently
from
7cbc43a
to
26ac77b
Compare
Added are 'inspect' and 'verify' options. Inspect makes sure the cache is consistent with itself and verify rebuilds trees and then verifies against the cached data (which can detect an error in the replica data).
cryptonemo
force-pushed
the
extend-tree-r-tool
branch
from
26ac77b
to
d507d8b
Compare
| pub type SectorShape32GiB = SectorShapeSub8; | ||
|
|
||
| pub type SectorShape32KiB = SectorShapeTop2; | ||
| pub type SectorShape64GiB = SectorShapeTop2; |
There was a problem hiding this comment.
Perfect, so much better. Thank you.
Added are 'inspect' and 'verify' options. Inspect makes sure the cache is consistent with itself and verify rebuilds trees and then verifies against the cached data (which can detect an error in the replica data).