Skip to content

Conversation

lrascao
Copy link

@lrascao lrascao commented Jun 8, 2017

linting, coverage, eunit tests, common tests
Also, do away with the custom tree calculation of the eager push list, use whatever the peer service gives us

@cmeiklejohn
Copy link
Member

Can you explain the change regarding the eager push list?

@lrascao
Copy link
Author

lrascao commented Jun 9, 2017

Sure, when i added the tests, specifically the connectedness check, i found that some times (not all) plumtree would not have a fully connected graph (considering both eager and lazy lists), this happened more often on 4-node clusters. I'm thinking this is a remnant of the basho fork (that was manually building the spanning tree) and does not make sense anymore now that the partisan dependency offers a correct peer sampling service.

@cmeiklejohn
Copy link
Member

Can you remove some of the debug logging? That's going to cause a significant performance hit if the code is executed often. (Unfortunately, even with a higher/lower log level, the debug code still gets dispatched.)

@lrascao
Copy link
Author

lrascao commented Jun 11, 2017

maybe have a switch to turn it on/off then? it's really hard tracking down anything in this without some logging
(edit: typos)

@lrascao
Copy link
Author

lrascao commented Jun 11, 2017

I've reworked it so now there's only debug when running tests

lrascao added 2 commits June 11, 2017 10:27
Declare a broadcast handler and using different
partisan managers test different configurations
under the same scenario: create a cluster, do
several rounds of broadcast on random nodes,
finally make one broadcast and ensure it reaches
the entire membership.
Instead of relying on a custom built tree use
the membership provided by partisan.
Directly add new members to the eager push
list as per the paper.
@cmeiklejohn cmeiklejohn merged commit 781c9df into lasp-lang:master Jun 11, 2017
@lrascao lrascao deleted the feature/ct branch July 2, 2017 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants