-
Notifications
You must be signed in to change notification settings - Fork 145
Fix pruning with consecutive finished dependencies #347
Conversation
This is from
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a real disapproval but wanted to raise awareness for the new attached log showing a HeaderNotFound
bug that was produced under this PR.
It also seems that, with this PR, I still run in that error where the
|
7b9e956
to
bb8881a
Compare
I'm having a very difficult time reproducing these. Sync is running for hours with nothing, I added a more extensive test in the meantime, and a bit more logging. Hopefully it will provide a bit more info the next time it happens. I think the best option is to merge, since this fixes a bug that happens more often. I'll be continuing to test and improve sync after merge. I'll leave the PR overnight and merge it in the morning if there are no objections. |
bb8881a
to
677a274
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm 👍 on merging this and track #377 individually
- The DB buffer should be smaller than the body request buffer, otherwise we'll never know if we have hit our maximum I/O throughput
677a274
to
bf26e30
Compare
What was wrong?
Fixes #344
I added a test to reproduce: if you set two chained finished dependencies, then try to prune the oldest one, it crashes because we didn't link one finished dependencies as a dependency of the other.
How was it fixed?
Check if the parent of a finished dependency is already present, and link it as a dependency, if so. Pruning works as expected, now.
Cute Animal Picture