-
Notifications
You must be signed in to change notification settings - Fork 189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
decaying connmgr tags for message delivery #328
Conversation
@vyzo the tag tracer is still missing "near first" message deliveries. Can you think of a good way to do this without duplicating the delivery record tracking from the score tracer? |
I think we can get away very simply by starting tracking in |
162538e
to
6cb821d
Compare
I squashed a few things and marked this as "ready for review", but we still need to wait until the dependency PRs are merged. |
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.
this looks good, just a comment.
Note: we'll want to go mod tidy
this when it's time to merge.
hmm, I added another 5 minutes to the travis timeout, but we're still hitting it. I can't imagine the tests in this branch take an extra 5m to run... I wonder if I could be deadlocking somewhere? |
okay, I finally wised up and figured out how to enable the race detector by default when I run tests in IntelliJ. With it enable, we hit the max goroutine limit in |
we should be getting output in that case. |
hmm, that's true. It did also take a few minutes on my laptop before it failed, so it might actually be taking more than 5m on travis |
I got the end-to-end test passing by lowering the number of test peers again, but now the unit test is failing, and it's a little confusing.
This is just after adding 50 minutes to the fake clock, and checking that the tag value has decreased. On my laptop, it gets the expected value, but it always seems to be off by one on Travis. The reason I'm confused is that the result should only depend on the fake clock we pass into the decayer, not the real clock. I tried sleeping the real clock for a second after bumping the fake one, to give the Decayer's processing routine time to run, but it still fails. I'm going to try increasing the sleep time to five seconds, but I'll probably be asleep myself by the time it's done running. Anyway, Vyzo, perhaps you could take a look if it's still failing. |
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.
Hard to tell what's happening with the tests on travis, code looks good to me.
64c58d1
to
7cd44aa
Compare
7cd44aa
to
a082669
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.
at last, green checkmaks!
This adds decaying connection manager tags that give a boost to peers that deliver messages. closes #306
I was hoping to finish and fix the test over the weekend, but didn't get a chance. It seems to basically work, but some of the honest peers aren't connected at the end of the test.
I'll sort it out in the morning, just wanted to get the branch up in case @vyzo wants to look through before then.