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

[CCR] Fail with a better error if leader index is red #35298

Merged
merged 6 commits into from
Nov 7, 2018

Conversation

martijnvg
Copy link
Member

when fetching history uuids from leader index as part of put follow and resume follow APIs.

as part of fetching history uuids from leader index.
@martijnvg martijnvg added >non-issue :Distributed/CCR Issues around the Cross Cluster State Replication features labels Nov 6, 2018
@martijnvg martijnvg requested a review from dnhatn November 6, 2018 14:05
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@@ -244,6 +244,11 @@ public void fetchLeaderHistoryUUIDs(
String leaderIndex = leaderIndexMetaData.getIndex().getName();
CheckedConsumer<IndicesStatsResponse, Exception> indicesStatsHandler = indicesStatsResponse -> {
IndexStats indexStats = indicesStatsResponse.getIndices().get(leaderIndex);
if (indexStats == null) {
onFailure.accept(new IllegalArgumentException("no index shards available, is the leader index red?"));
Copy link
Contributor

Choose a reason for hiding this comment

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

How about just saying - no index stats available for the leader index ? The leader index can be red when missing just one primary and you'd still have stats. Also maybe something else caused this and we're just misleading people? I prefer to keep this simple.

Copy link
Member

@dnhatn dnhatn 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 @martijnvg.

@martijnvg martijnvg merged commit e685cfe into elastic:master Nov 7, 2018
martijnvg added a commit that referenced this pull request Nov 7, 2018
as part of fetching history uuids from leader index.
martijnvg added a commit that referenced this pull request Nov 7, 2018
as part of fetching history uuids from leader index.
jasontedor added a commit to martijnvg/elasticsearch that referenced this pull request Nov 8, 2018
* master: (24 commits)
  Replicate index settings to followers (elastic#35089)
  Rename RealmConfig.globalSettings() to settings() (elastic#35330)
  [TEST] Cleanup FileUserPasswdStoreTests (elastic#35329)
  Scripting: Add back lookup vars in score script (elastic#34833)
  watcher: Fix integration tests to ensure correct start/stop of Watcher (elastic#35271)
  Remove ALL shard check in CheckShrinkReadyStep (elastic#35346)
  Use soft-deleted docs to resolve strategy for engine operation (elastic#35230)
  [ILM] Check shard and relocation status in AllocationRoutedStep (elastic#35316)
  Ignore date ranges containing 'now' when pre-processing a percolator query (elastic#35160)
  Add a frozen engine implementation (elastic#34357)
  Put a fake allocation id on allocate stale primary command (elastic#34140)
  [CCR] Enforce auto follow pattern name restrictions (elastic#35197)
  [ILM] rolling upgrade tests (elastic#35328)
  [ML] Add Missing data checking class (elastic#35310)
  Apply `ignore_throttled` also to concrete indices (elastic#35335)
  Make version field names more meaningful  (elastic#35334)
  [CCR] Added HLRC support for pause follow API (elastic#35216)
  [Docs] Improve Convert Processor description (elastic#35280)
  [Painless] Removes extraneous compile method (elastic#35323)
  [CCR] Fail with a better error if leader index is red (elastic#35298)
  ...
pgomulka pushed a commit to pgomulka/elasticsearch that referenced this pull request Nov 13, 2018
as part of fetching history uuids from leader index.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/CCR Issues around the Cross Cluster State Replication features >non-issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants