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

TransportVerifyShardBeforeCloseAction should force a flush #38401

Merged
merged 4 commits into from Feb 6, 2019

Conversation

Projects
None yet
4 participants
@tlrx
Copy link
Member

commented Feb 5, 2019

This commit changes the TransportVerifyShardBeforeCloseAction so that it always forces the flush of the shard. It seems that #37961 is not sufficient to ensure that the translog and the Lucene commit share the exact same max seq no and global checkpoint information in case of one or more noop operations have been made.

The BulkWithUpdatesIT.testThatMissingIndexDoesNotAbortFullBulkRequest and FrozenIndexTests.testFreezeEmptyIndexWithTranslogOps test this trivial situation and they both fail 1 on 10 executions.

Relates to #33888

@elasticmachine

This comment has been minimized.

Copy link

commented Feb 5, 2019

@ywelsch

ywelsch approved these changes Feb 5, 2019

@tlrx

This comment has been minimized.

Copy link
Member Author

commented Feb 5, 2019

@elasticmachine run elasticsearch-ci/2
@elasticmachine run elasticsearch-ci/1
@elasticmachine run elasticsearch-ci/default-distro

@tlrx

This comment has been minimized.

Copy link
Member Author

commented Feb 6, 2019

@ywelsch Sorry, I didn't notice among the recent CI failures that the restore REST YAML tests required to be adjusted with this change: now the flush is forced when closing the index, local files differs from the files that have been snapshotted and it shows up in the recovery stats. Can you please have a look at the changes in tests? Thanks

@tlrx tlrx requested a review from ywelsch Feb 6, 2019

@ywelsch

ywelsch approved these changes Feb 6, 2019

Copy link
Contributor

left a comment

LGTM

@tlrx tlrx merged commit 510829f into elastic:master Feb 6, 2019

7 checks passed

CLA Commit author is a member of Elasticsearch
Details
elasticsearch-ci/1 Build finished.
Details
elasticsearch-ci/2 Build finished.
Details
elasticsearch-ci/default-distro Build finished.
Details
elasticsearch-ci/docbldesx Build finished.
Details
elasticsearch-ci/oss-distro-docs Build finished.
Details
elasticsearch-ci/packaging-sample Build finished.
Details

@tlrx tlrx deleted the tlrx:always-force-flush branch Feb 6, 2019

@tlrx

This comment has been minimized.

Copy link
Member Author

commented Feb 6, 2019

Thanks @ywelsch

tlrx added a commit to tlrx/elasticsearch that referenced this pull request Feb 6, 2019

TransportVerifyShardBeforeCloseAction should force a flush (elastic#3…
…8401)

This commit changes the `TransportVerifyShardBeforeCloseAction` so that it 
always forces the flush of the shard. It seems that elastic#37961 is not sufficient to 
ensure that the translog and the Lucene commit share the exact same max 
seq no and global checkpoint information in case of one or more noop 
operations have been made.

The `BulkWithUpdatesIT.testThatMissingIndexDoesNotAbortFullBulkRequest` 
and `FrozenIndexTests.testFreezeEmptyIndexWithTranslogOps` test this trivial 
situation and they both fail 1 on 10 executions.

Relates to elastic#33888

tlrx added a commit that referenced this pull request Feb 6, 2019

TransportVerifyShardBeforeCloseAction should force a flush (#38401)
This commit changes the `TransportVerifyShardBeforeCloseAction` so that it 
always forces the flush of the shard. It seems that #37961 is not sufficient to 
ensure that the translog and the Lucene commit share the exact same max 
seq no and global checkpoint information in case of one or more noop 
operations have been made.

The `BulkWithUpdatesIT.testThatMissingIndexDoesNotAbortFullBulkRequest` 
and `FrozenIndexTests.testFreezeEmptyIndexWithTranslogOps` test this trivial 
situation and they both fail 1 on 10 executions.

Relates to #33888

@colings86 colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Feb 11, 2019

Merge branch 'master' into retention-lease-ccr
* master: (27 commits)
  Mute AnalysisModuleTests#testStandardFilterBWC (elastic#38636)
  add geotile_grid ref to asciidoc (elastic#38632)
  Enable Dockerfile from artifacts.elastic.co (elastic#38552)
  Mute FollowerFailOverIT testFailOverOnFollower (elastic#38634)
  Account for a possible rolled over file while reading the audit log file (elastic#34909)
  Mute failure in InternalEngineTests (elastic#38622)
  Fix Issue with Concurrent Snapshot Init + Delete (elastic#38518)
  Refactor ZonedDateTime.now in millis resolution (elastic#38577)
  Mute failing WatchStatusIntegrationTests (elastic#38621)
  Mute failing  ApiKeyIntegTests (elastic#38614)
  [DOCS] Add warning about bypassing ML PUT APIs (elastic#38509)
  Add 7.1 and 8.0 version constants to master (elastic#38514)
  ML: update set_upgrade_mode, add logging (elastic#38372)
  bad formatted JSON object (elastic#38515) (elastic#38525)
  Fix HistoryIntegrationTests timestamp comparsion (elastic#38505)
  SQL: Fix issue with IN not resolving to underlying keyword field (elastic#38440)
  Fix the clock resolution to millis in ScheduledEventTests (elastic#38506)
  Enable BWC after backport recovering leases (elastic#38485)
  Collapse retention lease integration tests (elastic#38483)
  TransportVerifyShardBeforeCloseAction should force a flush (elastic#38401)
  ...

dimitris-athanasiou added a commit to dimitris-athanasiou/elasticsearch that referenced this pull request Feb 12, 2019

TransportVerifyShardBeforeCloseAction should force a flush (elastic#3…
…8401)

This commit changes the `TransportVerifyShardBeforeCloseAction` so that it 
always forces the flush of the shard. It seems that elastic#37961 is not sufficient to 
ensure that the translog and the Lucene commit share the exact same max 
seq no and global checkpoint information in case of one or more noop 
operations have been made.

The `BulkWithUpdatesIT.testThatMissingIndexDoesNotAbortFullBulkRequest` 
and `FrozenIndexTests.testFreezeEmptyIndexWithTranslogOps` test this trivial 
situation and they both fail 1 on 10 executions.

Relates to elastic#33888
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.