Skip to content

Only filter intial recovery (post API) when shrinking an index#18661

Merged
s1monw merged 3 commits intoelastic:masterfrom
s1monw:filter_initial_recover
Jun 2, 2016
Merged

Only filter intial recovery (post API) when shrinking an index#18661
s1monw merged 3 commits intoelastic:masterfrom
s1monw:filter_initial_recover

Conversation

@s1monw
Copy link
Copy Markdown
Contributor

@s1monw s1monw commented May 31, 2016

Today we use index.routing.allocation.include._id to filter the allocation
for the shrink target index. That has the sideeffect that the user has to
delete that setting / change it once the primary has been recovered (shrink is done)
This PR adds a dedicated filter that can only be set internally that only filters
allocation for unassigned shards.

@ywelsch do you wanna take a look?

Today we use `index.routing.allocation.include._id` to filter the allocation
for the shrink target index. That has the sideeffect that the user has to
delete that setting / change it once the primary has been recovered (shrink is done)
This PR adds a dedicated filter that can only be set internally that only filters
allocation for unassigned shards.
@s1monw
Copy link
Copy Markdown
Contributor Author

s1monw commented May 31, 2016

hmm I guess I should have a unittest for this as well... adding...

@s1monw
Copy link
Copy Markdown
Contributor Author

s1monw commented May 31, 2016

@ywelsch I added a test - do you mind looking at it

.put("index.routing.allocation.include._id",
// we use "i.r.a.initial_recovery" rather than "i.r.a.require|include" since we want the replica to allocate right away
// once we are allocated.
.put("index.routing.allocation.initial_recovery._id",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could remove this setting once all shards have been activated (in AllocationService.updateMetaDataWithRoutingTable()).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am not sure it's worth the trouble? i kind of like the history we are getting with settings like this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok, I understand that. I also don't like the added complexity of having this removal code in another place...

@ywelsch
Copy link
Copy Markdown
Contributor

ywelsch commented Jun 1, 2016

Left minor comments and wonder if we should clean up the private setting once all shards have been activated.

@s1monw
Copy link
Copy Markdown
Contributor Author

s1monw commented Jun 1, 2016

@ywelsch pushed a new commit

@ywelsch
Copy link
Copy Markdown
Contributor

ywelsch commented Jun 1, 2016

LGTM. Thanks @s1monw!

@s1monw s1monw merged commit 22dfc41 into elastic:master Jun 2, 2016
jasontedor added a commit to rjernst/elasticsearch that referenced this pull request Jun 3, 2016
* master: (911 commits)
  [TEST] wait for yellow after setup doc tests (elastic#18726)
  Fix recovery throttling to properly handle relocating non-primary shards (elastic#18701)
  Fix merge stats rendering in RestIndicesAction (elastic#18720)
  [TEST] mute RandomAllocationDeciderTests.testRandomDecisions
  Reworked docs for index-shrink API (elastic#18705)
  Improve painless compile-time exceptions
  Adds UUIDs to snapshots
  Add test rethrottle test case for delete-by-query
  Do not start scheduled pings until transport start
  Adressing review comments
  Only filter intial recovery (post API) when shrinking an index (elastic#18661)
  Add tests to check that toQuery() doesn't return null
  Removing handling of null lucene query where we catch this at parse time
  Handle empty query bodies at parse time and remove EmptyQueryBuilder
  Mute failing assertions in IndexWithShadowReplicasIT until fix
  Remove allow running as root
  Add upgrade-not-supported warning to alpha release notes
  remove unrecognized javadoc tag from matrix aggregation module
  set ValuesSourceConfig fields as private
  Adding MultiValuesSource support classes and documentation to matrix stats agg module
  ...
@lcawl lcawl added :Distributed/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. and removed :Allocation labels Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. >enhancement v5.0.0-alpha4

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants