Skip to content
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

Separate new tipset handling from message pool implementation, wrap in an inbox. #2798

Merged
merged 1 commit into from May 29, 2019

Conversation

@anorth
Copy link
Contributor

commented May 20, 2019

With this change, the incoming and outbound message queue/pool follow a similar pattern.
They're not quite the same, since validation is performed inside the message pool but
prior to the outbound queue (which we could also align).

@anorth anorth requested review from acruikshank and rosalinekarr May 20, 2019

@anorth anorth force-pushed the anorth/sender-2b branch from 57dfc2f to acae2ac May 20, 2019

@rosalinekarr
Copy link
Contributor

left a comment

LGTM, nothing blocking

cids = append(cids, c)
}
}
return cids

This comment has been minimized.

Copy link
@rosalinekarr

rosalinekarr May 20, 2019

Contributor

Not something we need to address in this PR, but we could get a big performance improvement here by sorting pending upon insertion and exiting early here. It might be a good idea to add an icebox task for optimizing this.

// those from the removed chain (if any) that do not appear in the new chain.
// We think that the right model for keeping the message pool up to date is
// to think about it like a garbage collector.
func (ib *Inbox) HandleNewHead(ctx context.Context, oldHead, newHead types.TipSet) error {

This comment has been minimized.

Copy link
@anorth

anorth May 21, 2019

Author Contributor

This is copied verbatim from message_pool.go UpdateMessagePool

"github.com/stretchr/testify/require"
)

func TestUpdateMessagePool(t *testing.T) {

This comment has been minimized.

Copy link
@anorth

anorth May 21, 2019

Author Contributor

These tests are copied from message_pool_test, with minor dependency changes.

@anorth anorth force-pushed the anorth/sender branch 2 times, most recently from 8069cce to fcb2cfe May 22, 2019

@anorth anorth force-pushed the anorth/sender-2b branch from acae2ac to a7bf1ac May 22, 2019

@anorth anorth changed the base branch from anorth/sender to master May 22, 2019

@anorth

This comment has been minimized.

Copy link
Contributor Author

commented May 26, 2019

@acruikshank
Copy link
Contributor

left a comment

Haven't had a chance to go through this in as much detail as I'd like. The idea seems good. Since it's a refactor and the testing seems to be in order, LGTM.

@anorth anorth force-pushed the anorth/sender-2b branch from a7bf1ac to ce0e46a May 29, 2019

Separate new tipset handling from message pool implementation, wrap i…
…n an inbox.

With this change, the incoming and outbound message queue/pool follow a similar pattern.
They're not quite the same, since validation is performed inside the message pool but
prior to the outbound queue (which we could also align).

@anorth anorth force-pushed the anorth/sender-2b branch from ce0e46a to 11f77e2 May 29, 2019

@anorth anorth merged commit 3927793 into master May 29, 2019

5 checks passed

ci/circleci: build_linux-1 Your tests passed on CircleCI!
Details
ci/circleci: deps_linux Your tests passed on CircleCI!
Details
ci/circleci: functional_test_linux-1 Your tests passed on CircleCI!
Details
ci/circleci: integration_test_linux Your tests passed on CircleCI!
Details
ci/circleci: unit_test_linux Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.