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

TESTS+DISTR.: Fix testIndexCheckOnStartup Flake #33349

Merged
merged 1 commit into from
Sep 3, 2018

Conversation

original-brownbear
Copy link
Member

@original-brownbear original-brownbear commented Sep 3, 2018

  • Ignore all RuntimeException since random
    file corruption triggers other RTE in addition
    to the randomly caught one
    • In the reported case it throws "java.lang.IllegalArgumentException: Illegal field number: -65281"
    • -> It's probably fine to just suppress here instead of trying to foresee+catch all possible exceptions that a randomly corrupted file can trigger?
  • closes [CI] IndexShardTests#testIndexCheckOnStartup fails #33345

* Ignore all `RuntimeException` since random
file corruption triggers other RTE in addition
to the randomly caught one
* closes elastic#33345
@original-brownbear original-brownbear added >test-failure Triaged test failures from CI :Distributed Indexing/Store Issues around managing unopened Lucene indices. If it touches Store.java, this is a likely label. v7.0.0 v6.5.0 labels Sep 3, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

Copy link
Contributor

@vladimirdolzhenko vladimirdolzhenko left a comment

Choose a reason for hiding this comment

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

LGTM

I was thinking about disabling check index on close with
BaseDirectoryWrapper :

IndexShard corruptedShard = newShard(shardRouting, shardPath, indexMetaData,
            is -> {
                final ShardId shardId = indexShard.shardId;
                final IndexSettings indexSettings = indexShard.indexSettings();
                DirectoryService directoryService = new DirectoryService(shardId, indexSettings) {
                    @Override
                    public Directory newDirectory() throws IOException {
                        final BaseDirectoryWrapper baseDirectoryWrapper = newFSDirectory(shardPath.resolveIndex());
                        baseDirectoryWrapper.setCheckIndexOnClose(false);
                        return baseDirectoryWrapper;
                    }
                };
                return new Store(shardId, indexSettings, directoryService, new DummyShardLock(shardId));
            }, null, indexShard.engineFactory,
            indexShard.getGlobalCheckpointSyncer(), EMPTY_EVENT_LISTENER);

but catching RuntimeException is fine here and way simplier

@original-brownbear
Copy link
Member Author

@vladimirdolzhenko thanks, will merge once green :)

@original-brownbear original-brownbear merged commit 42424af into elastic:master Sep 3, 2018
@original-brownbear original-brownbear deleted the 33345 branch September 3, 2018 15:06
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Sep 3, 2018
* Ignore all `RuntimeException` since random
file corruption triggers other RTE in addition
to the randomly caught one
* closes elastic#33345
original-brownbear added a commit that referenced this pull request Sep 3, 2018
* Ignore all `RuntimeException` since random
file corruption triggers other RTE in addition
to the randomly caught one
* closes #33345
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Sep 4, 2018
…e-default-distribution

* elastic/master: (213 commits)
  ML: Fix build after HLRC change
  Fix inner hits retrieval when stored fields are disabled (_none_) (elastic#33018)
  SQL: Show/desc commands now support table ids (elastic#33363)
  Mute testValidateFollowingIndexSettings
  HLRC: Add delete by query API (elastic#32782)
  [ML] The sort field on get records should default to the record_score (elastic#33358)
  [ML] Minor improvements to categorization Grok pattern creation (elastic#33353)
  [DOCS] fix a couple of typos (elastic#33356)
  Disable assemble task instead of removing it (elastic#33348)
  Simplify the return type of FieldMapper#parse. (elastic#32654)
  [ML] Delete forecast API (elastic#31134) (elastic#33218)
  Introduce private settings (elastic#33327)
  [Docs] Add search timeout caveats (elastic#33354)
  TESTS: Fix Race Condition in Temp Path Creation (elastic#33352)
  Fix from_range in search_after in changes snapshot (elastic#33335)
  TESTS+DISTR.: Fix testIndexCheckOnStartup Flake (elastic#33349)
  Null completion field should not throw IAE (elastic#33268)
  Adds code to help with IndicesRequestCacheIT failures (elastic#33313)
  Prevent NPE parsing the stop datafeed request. (elastic#33347)
  HLRC: Add ML get overall buckets API (elastic#33297)
  ...
@tomcallahan tomcallahan added >test Issues or PRs that are addressing/adding tests and removed >test-failure Triaged test failures from CI labels Sep 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Indexing/Store Issues around managing unopened Lucene indices. If it touches Store.java, this is a likely label. >test Issues or PRs that are addressing/adding tests v6.5.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] IndexShardTests#testIndexCheckOnStartup fails
5 participants