A relay that has answered nothing cannot be asked for the last hour - #204
Merged
Conversation
#197 dials relays the reader is not on, chosen because the people they follow write there, and asks each about exactly those people. It asked them with a `since`, and that made the whole feature deliver almost nothing. Shipped an hour ago in v0.3.0. `feedSince` is "the newest note I hold, minus an hour". On the pool's own relays that is right: they have been answering this same question all along, so anything older is already in the store, and the bound is what stops every reconnect re-downloading the backlog. A routed relay has answered nothing. It was dialled precisely because it holds notes from people whose posts the reader has never had, and every one of those is older than the newest note the reader holds from anybody else. So the bound asked a relay full of missing history for the last hour of it. Amethyst hit this from the other side and wrote it down: a `since` floor "silently emptied the tab" on a cold start. Routed subscriptions are built by their own function now, so the two questions cannot be conflated again by editing one call site, and the reason is at the top of it. Live, on my own account, before and after: with the bound, effectively nothing. Without it, three routed relays asked about 29, 22 and 18 people delivered more than six hundred notes in a hundred seconds. The test needed a second try and that is the more useful half of this. The first version passed with the bug still in, because `feedSince` returns null when the feed holds no note at all, which is every test. It asserted "no since" against code that could not have produced one. It stamps a newest note first now, and asserts `feedSince` is non-null before asserting the filters do not carry it, so the fix is what makes it pass. Verified by putting the bug back and watching it fail.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#197 dials relays the reader is not on, chosen because the people they follow write there, and asks each about exactly those people. It asked them with a
since, and that made the whole feature deliver almost nothing. Shipped an hour ago in v0.3.0.feedSinceis the newest note I hold, minus an hour. On the pool's own relays that is correct: they have been answering this same question all along, so anything older is already in the store, and the bound is what stops every reconnect re-downloading the backlog.A routed relay has answered nothing. It was dialled precisely because it holds notes from people whose posts the reader has never had, and every one of those is older than the newest note the reader holds from anybody else. So the bound asked a relay full of missing history for the last hour of it.
Amethyst hit this from the other side and wrote it down: a
sincefloor "silently emptied the tab" on a cold start.Routed subscriptions get their own builder now, so the two questions cannot be conflated again by editing one call site, and the reasoning sits at the top of it.
Live, before and after
Same account, same relays. With the bound: effectively nothing. Without it, three routed relays asked about 29, 22 and 18 people delivered more than six hundred notes in a hundred seconds.
The test needed a second try, which is the more useful half
The first version passed with the bug still in.
feedSincereturns null when the feed holds no note at all, which is the case in every test, so it was asserting "no since" against code that could not have produced one. Removing the fix failed nothing.It now stamps a newest note first and asserts
feedSinceis non-null before asserting the filters do not carry it, so the fix is what makes it pass. Verified by putting the bug back and watching it go red.That is the third time today a test of mine passed for the wrong reason, and the second where the tell was "the probe failed nothing".