Skip to content

Conversation

@original-brownbear
Copy link
Contributor

@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
Contributor 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