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

Index service test suite should go through the system #24491

Closed
jasontedor opened this issue May 4, 2017 · 4 comments
Closed

Index service test suite should go through the system #24491

jasontedor opened this issue May 4, 2017 · 4 comments
Labels
:Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. >test Issues or PRs that are addressing/adding tests

Comments

@jasontedor
Copy link
Member

I discovered this while working on inlining global checkpoints. The IndexServiceTests#testRescheduleAsyncFsync test failed on my branch. The idea behind this test is that we set the translog durability to async, index a document, and then busily check that the translog does not need to be synced. The expectation is that an async fsync task should fire and at some point fsync the translog. This test was passing on master, failing on my branch. It was passing on master for the wrong reason. The problem is that setting durability to async never stuck but were instead on request durability. Thus, a request fsync had already fired so it did not matter if an async fsync never fired. The details of exactly why are explained in cb46e97. The reason this fails on my branch is because with inlining global checkpoints, the translog does indeed need another fsync after the replication operation completes because we persist the global checkpoint into the translog checkpoint. Since we were stuck on request durability, this fsync never fired and the test failed.

I pushed cb46e97 to fix this test so the test goes through the system and the async durability setting sticks. There are other tests in this test suite doing the same thing. I think that should be updated, but I would rather spend my immediate time on inlining global checkpoints so this issue is a placeholder for addressing these tests in the future.

@jasontedor jasontedor added the >test Issues or PRs that are addressing/adding tests label May 4, 2017
@colings86 colings86 added the :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. label Apr 24, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@bleskes
Copy link
Contributor

bleskes commented Apr 24, 2018

@dnhatn do you mind spending some time triaging this and seeing what needs to be done?

@dnhatn
Copy link
Member

dnhatn commented Apr 24, 2018

@bleskes Looking now.

dnhatn added a commit to dnhatn/elasticsearch that referenced this issue Apr 24, 2018
Today we update index settings directly via IndexService instead of the
cluster state in IndexServiceTests. However, those changes will be lost
if there is a cluster state update. In general, we should update index
settings via client and limit the direct usage in only special tests.

This commit replaces direct usages by the updateSettings api of client.

Closes elastic#24491
@dnhatn
Copy link
Member

dnhatn commented Apr 24, 2018

I opened #29682.

dnhatn added a commit that referenced this issue Apr 26, 2018
Today we update index settings directly via IndexService instead of the
cluster state in IndexServiceTests. However, those changes will be lost
if there is a cluster state update. In general, we should update index
settings via client and limit the direct usage in only special tests.

This commit replaces direct usages by the updateSettings api of client.

Closes #24491
dnhatn added a commit that referenced this issue Apr 26, 2018
Today we update index settings directly via IndexService instead of the
cluster state in IndexServiceTests. However, those changes will be lost
if there is a cluster state update. In general, we should update index
settings via client and limit the direct usage in only special tests.

This commit replaces direct usages by the updateSettings api of client.

Closes #24491
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. >test Issues or PRs that are addressing/adding tests
Projects
None yet
Development

No branches or pull requests

5 participants