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

[CI] DocumentSubsetReaderTests testSearch fails (seed-specific) #32457

Closed
droberts195 opened this issue Jul 30, 2018 · 5 comments
Closed

[CI] DocumentSubsetReaderTests testSearch fails (seed-specific) #32457

droberts195 opened this issue Jul 30, 2018 · 5 comments
Assignees
Labels
:Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC >test-failure Triaged test failures from CI v6.5.0 v7.0.0-beta1

Comments

@droberts195
Copy link
Contributor

There is a seed-specific failure in the DocumentSubsetReaderTests testSearch test.

This has failed on both master and 6.x.

Master:
https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+periodic/6761/console
6.x:
https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+6.x+periodic/2457/console

It's reproducible using this command:

./gradlew :x-pack:plugin:core:test \
  -Dtests.seed=4CDD600B7538DB55 \
  -Dtests.class=org.elasticsearch.xpack.core.security.authz.accesscontrol.DocumentSubsetReaderTests \
  -Dtests.method="testSearch" \
  -Dtests.security.manager=true \
  -Dtests.locale=fi-FI \
  -Dtests.timezone=Europe/Kiev

It doesn't reproduce with other seeds.

I will mute the test on master and 6.x

@droberts195 droberts195 added >test-failure Triaged test failures from CI v7.0.0 :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC v6.5.0 labels Jul 30, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

droberts195 added a commit that referenced this issue Jul 30, 2018
droberts195 added a commit that referenced this issue Jul 30, 2018
@tvernum tvernum self-assigned this Jul 30, 2018
@tvernum
Copy link
Contributor

tvernum commented Jul 30, 2018

This seems to be a result of the upgrade to Lucene 7.5.0 (#32390 / 53ff06e).

I'll debug further to work out if it's just a test bug or a genuine behaviour change.

FYI @jimczi ; I'll let you know what I find.

@tvernum
Copy link
Contributor

tvernum commented Jul 30, 2018

@jimczi (or another Lucene expert)

I think I understand why this test is failing and can guess at why it is triggered in Lucene 7.5, but I'm not sure of the best way to fix it.

The problem is between lines 102 and 104 here:

Depending on the merge policy/schedule configuration (which is randomised in LuceneTestCase), it's possible that the delete would trigger a merge, which causes the "value3" document to be removed entirely, and the "value4" document become the doc with index 2 instead of 3 which fails the assertion at line 133.

Since Lucene 7.5 contains reclaim_deletes_weight/setDeletesPctAllowed changes, and this index has so few docs, a single delete could trigger a merge.

If that analysis is correct, what's the cleanest way to ensure that no merge happens when the document is deleted at line 103?

@jimczi
Copy link
Contributor

jimczi commented Jul 30, 2018

Good catch @tvernum ! One workaround is to force the merge policy to choose a log merge policy (this policy does not handle the new setDeletesPctAllowed) like this:

    IndexWriter iw = new IndexWriter(directory, newIndexWriterConfig().setMergePolicy(newLogMergePolicy(random()));

It seems that the test only need a single segment with a delete so it could also work if you disable the merge entirely and set a big buffer size for the index writer. Though the solution with the log merge policy should work fine. Thanks for chasing this up Tim !

@dnhatn
Copy link
Member

dnhatn commented Jul 30, 2018

@tvernum and @jimczi Another workaround is to index more documents to reduce the deletion ratio.

@jimczi jimczi closed this as completed in 996ed73 Aug 16, 2018
jimczi added a commit that referenced this issue Aug 17, 2018
The merge policy that was used could lead to unpredictive merges due to the
randomization of `setDeletesPctAllowed`.

Closes #32457
jimczi added a commit that referenced this issue Feb 21, 2019
The merge policy that was used could lead to unpredictive merges due to the
randomization of `setDeletesPctAllowed`.

Closes #32457
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC >test-failure Triaged test failures from CI v6.5.0 v7.0.0-beta1
Projects
None yet
Development

No branches or pull requests

6 participants