perf(store): pass batch through context #328
Conversation
Wondertan
requested review from
tzdybal,
renaynay,
vgonkivs,
gupadhyaya and
walldiss
as code owners
|
should we wait until these changes are merged and released in ipfs? |
|
|
Retesting this with all the recent changes showed significant performance degradation. The test showed The best measurement in the description ( |
|
False alarm. I triple checked, and there was a bug on the integration side in the node, which made this change effectively a noop. The results after the fix are as expected, around ~120s. |
This PR shaves another 30s when we prune the whole chain. It achieves this by applying the same technique as in #328, but for reads. Previously, the underlying datastore would create a read-only transaction for every singular read. As shown in pyroscope profiles, this puts pressure on badger as creating new transactions ain't cheap and involves complex synchronization to ensure atomicity between parallel transactions. With this insight in mind, I also integrated read batching in other places where this must be beneficial: advancing, receding, range, and singular header reads. More noticeable changes: * Fixes a silent bug that wouldn't delete samples on the Pruner side * Fixes a bug in the upstream contextds change * Adds tracking for missing headers that were not found during deletion
Allows batch deleting across API boundaries.
Depends on #320
Depends on ipfs/go-datastore#238, but we shouldn't block on it being released upstream. It should be fine to temporarily live with the replace so rollout pruning on time
w/o:
{"from_height": 1, "to_height": 6500189, "took": 341.992917359}w/:
{"from_height": 1, "to_height": 6500617, "took": 129.445211431}