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

Inline TransportReplAction#registerRequestHandlers #40762

Conversation

DaveCTurner
Copy link
Contributor

It is important that resync actions are not rejected on the primary even if its
write threadpool is overloaded. Today we do this by exposing
registerRequestHandlers to subclasses and overriding it in
TransportResyncReplicationAction. This isn't ideal because it obscures the
difference between this action and other replication actions, and also might
allow subclasses to try and use some state before they are properly
initialised. This change replaces this override with a constructor parameter to
solve these issues.

Relates #40706.

It is important that resync actions are not rejected on the primary even if its
`write` threadpool is overloaded. Today we do this by exposing
`registerRequestHandlers` to subclasses and overriding it in
`TransportResyncReplicationAction`. This isn't ideal because it obscures the
difference between this action and other replication actions, and also might
allow subclasses to try and use some state before they are properly
initialised. This change replaces this override with a constructor parameter to
solve these issues.

Relates elastic#40706
@DaveCTurner DaveCTurner added :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >refactoring v8.0.0 v7.2.0 labels Apr 3, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Thanks @DaveCTurner

@DaveCTurner DaveCTurner merged commit c29cebb into elastic:master Apr 3, 2019
DaveCTurner added a commit that referenced this pull request Apr 3, 2019
It is important that resync actions are not rejected on the primary even if its
`write` threadpool is overloaded. Today we do this by exposing
`registerRequestHandlers` to subclasses and overriding it in
`TransportResyncReplicationAction`. This isn't ideal because it obscures the
difference between this action and other replication actions, and also might
allow subclasses to try and use some state before they are properly
initialised. This change replaces this override with a constructor parameter to
solve these issues.

Relates #40706
@DaveCTurner DaveCTurner deleted the 2019-04-03-transport-replication-action-inline-registerRequestHandlers branch April 3, 2019 13:37
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Apr 3, 2019
…leniency

* elastic/master:
  SQL: Fix deserialisation issue of TimeProcessor (elastic#40776)
  Improve GCS docs for using keystore (elastic#40605)
  Add Restore Operation to SnapshotResiliencyTests (elastic#40634)
  Small refactorings to analysis components (elastic#40745)
  SQL: Fix display size for DATE/DATETIME (elastic#40669)
  add HLRC protocol tests for transform state and stats (elastic#40766)
  Inline TransportReplAction#registerRequestHandlers (elastic#40762)
  remove experimental label from search_as_you_type documentation (elastic#40744)
  Remove some abstractions from `TransportReplicationAction` (elastic#40706)
  Upgrade to latest build scan plugin (elastic#40702)
  Use default memory lock setting in testing (elastic#40730)
  Add Bulk Delete Api to BlobStore (elastic#40322)
  Remove yaml skips older than 7.0 (elastic#40183)
  Docs: Move id in the java-api (elastic#40748)
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
It is important that resync actions are not rejected on the primary even if its
`write` threadpool is overloaded. Today we do this by exposing
`registerRequestHandlers` to subclasses and overriding it in
`TransportResyncReplicationAction`. This isn't ideal because it obscures the
difference between this action and other replication actions, and also might
allow subclasses to try and use some state before they are properly
initialised. This change replaces this override with a constructor parameter to
solve these issues.

Relates elastic#40706
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >refactoring v7.2.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants