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

Tribe nodes should apply cluster state updates in batches #14993

Merged
merged 1 commit into from Dec 16, 2015

Conversation

Projects
None yet
2 participants
@jasontedor
Member

jasontedor commented Nov 25, 2015

This commit applies the general mechanism for applying cluster state updates in batches to tribe nodes.

Relates #14899, relates #14725

@jasontedor

This comment has been minimized.

Show comment
Hide comment
@jasontedor

jasontedor Nov 25, 2015

Member

@bleskes I'll rebase this pull request on master when #14899 is reintegrated there. The salient commit for this review is thus 476ab3c91b038f5a328bf9360c87e4ee792643d0 pending #14899 (all the changes for that commit are in TribeService.java).

Member

jasontedor commented Nov 25, 2015

@bleskes I'll rebase this pull request on master when #14899 is reintegrated there. The salient commit for this review is thus 476ab3c91b038f5a328bf9360c87e4ee792643d0 pending #14899 (all the changes for that commit are in TribeService.java).

@jasontedor jasontedor added the v2.2.0 label Nov 25, 2015

@jasontedor

This comment has been minimized.

Show comment
Hide comment
@jasontedor

jasontedor Nov 26, 2015

Member

@bleskes I've rebased this pull request on the latest changes from #14899.

Member

jasontedor commented Nov 26, 2015

@bleskes I've rebased this pull request on the latest changes from #14899.

@jasontedor

This comment has been minimized.

Show comment
Hide comment
@jasontedor

jasontedor Dec 1, 2015

Member

@bleskes This pull request has been rebased on master since #14899 has been integrated there.

Member

jasontedor commented Dec 1, 2015

@bleskes This pull request has been rebased on master since #14899 has been integrated there.

@bleskes

This comment has been minimized.

Show comment
Hide comment
@bleskes

bleskes Dec 3, 2015

Member

I think we can go further with this one. Now it just saves on cluster state update task, but the work done is the same. I think each executor can pick the latest task (double check that batching maintains order, it should :)) and only apply that.

We could in theory also have a global executor (i.e., for all clients) which takes the tasks, groups them by source cluster and applies them to a shared builder but then we run the risk of exceptions in one client blocking updates from all client. I don't think it's worth it.

Member

bleskes commented Dec 3, 2015

I think we can go further with this one. Now it just saves on cluster state update task, but the work done is the same. I think each executor can pick the latest task (double check that batching maintains order, it should :)) and only apply that.

We could in theory also have a global executor (i.e., for all clients) which takes the tasks, groups them by source cluster and applies them to a shared builder but then we run the risk of exceptions in one client blocking updates from all client. I don't think it's worth it.

@jasontedor

This comment has been minimized.

Show comment
Hide comment
@jasontedor

jasontedor Dec 14, 2015

Member

I think we can go further with this one.

@bleskes Addressed in 74adaca4cebd9c0fde32a5080d7cfbbf2dbe9746.

Member

jasontedor commented Dec 14, 2015

I think we can go further with this one.

@bleskes Addressed in 74adaca4cebd9c0fde32a5080d7cfbbf2dbe9746.

@jasontedor

This comment has been minimized.

Show comment
Hide comment
@jasontedor

jasontedor Dec 15, 2015

Member

@bleskes In addition to 74adaca4cebd9c0fde32a5080d7cfbbf2dbe9746, I also pushed 479e50adb26b2f5453841273c05cdb17c530065e to only return a new cluster state instance in the tribe service if there were changes to the cluster state.

Member

jasontedor commented Dec 15, 2015

@bleskes In addition to 74adaca4cebd9c0fde32a5080d7cfbbf2dbe9746, I also pushed 479e50adb26b2f5453841273c05cdb17c530065e to only return a new cluster state instance in the tribe service if there were changes to the cluster state.

@bleskes

This comment has been minimized.

Show comment
Hide comment
@bleskes

bleskes Dec 16, 2015

Member

LGTM. Double checking - do we have a test that makes sure tasks are passed in order?

Member

bleskes commented Dec 16, 2015

LGTM. Double checking - do we have a test that makes sure tasks are passed in order?

@jasontedor

This comment has been minimized.

Show comment
Hide comment
@jasontedor

jasontedor Dec 16, 2015

Member

LGTM. Double checking - do we have a test that makes sure tasks are passed in order?

I don't think so; should we address that in a separate issue?

Member

jasontedor commented Dec 16, 2015

LGTM. Double checking - do we have a test that makes sure tasks are passed in order?

I don't think so; should we address that in a separate issue?

@bleskes

This comment has been minimized.

Show comment
Hide comment
@bleskes

bleskes Dec 16, 2015

Member

I don't think so; should we address that in a separate issue?

++ on another issue.. It's an important semantics. We should test it under heavy concurrency.

Member

bleskes commented Dec 16, 2015

I don't think so; should we address that in a separate issue?

++ on another issue.. It's an important semantics. We should test it under heavy concurrency.

@jasontedor

This comment has been minimized.

Show comment
Hide comment
@jasontedor

jasontedor Dec 16, 2015

Member

++ on another issue.. It's an important semantics. We should test it under heavy concurrency.

I opened #15483 to track this.

Member

jasontedor commented Dec 16, 2015

++ on another issue.. It's an important semantics. We should test it under heavy concurrency.

I opened #15483 to track this.

Tribe nodes should apply cluster state updates in batches
This commit applies the general mechanism for applying cluster state
updates in batches to tribe nodes.

Relates #14899, relates #14725

jasontedor added a commit that referenced this pull request Dec 16, 2015

Merge pull request #14993 from jasontedor/tribe-node-cluster-state-batch
Tribe nodes should apply cluster state updates in batches

@jasontedor jasontedor merged commit 709740e into elastic:master Dec 16, 2015

1 check passed

CLA Commit author is a member of Elasticsearch
Details

@jasontedor jasontedor deleted the jasontedor:tribe-node-cluster-state-batch branch Dec 16, 2015

@jasontedor

This comment has been minimized.

Show comment
Hide comment
@jasontedor

jasontedor Dec 16, 2015

Member

Integrated to master in 709740e and backported to 2.x in 8fa5c68.

Member

jasontedor commented Dec 16, 2015

Integrated to master in 709740e and backported to 2.x in 8fa5c68.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment