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

Make a test in ClientTest more consistent #634

Merged
merged 1 commit into from Jun 13, 2014
Merged

Conversation

nik9000
Copy link
Contributor

@nik9000 nik9000 commented Jun 13, 2014

ClientTest->testDeleteIdsIdxStringTypeString assumed that new documents
created without a specified id or routing value would route to the same
shard as "1" and a different shard from "2" when there are two shards.
This wasn't always that case because:

  1. Documents without a routing value use their ids as a route.
  2. Documents that don't specify an id get a random one.

This sets the routing for the document to "1" which, luckily, does in
fact route to a different shard then "2" when there are two shards. And
it will stay that way forever back Elasticsearch has to maintain a consistent
routing style or it won't be safe to upgrade.

Closes #633

ClientTest->testDeleteIdsIdxStringTypeString assumed that new documents
created without a specified id or routing value would route to the same
shard as "1" and a different shard from "2" when there are two shards.
This wasn't always that case because:
1.  Documents without a routing value use their ids as a route.
2.  Documents that don't specify an id get a random one.

This sets the routing for the document to "1" which, luckily, does in
fact route to a different shard then "2" when there are two shards.  And
it will stay that way forever back Elasticsearch has to maintain a consistent
routing style or it won't be safe to upgrade.

Closes ruflin#633
@nik9000
Copy link
Contributor Author

nik9000 commented Jun 13, 2014

I'm actually not sure how it worked before this. I didn't go digging into history, only Elasticsearch's source code.

@ruflin
Copy link
Owner

ruflin commented Jun 13, 2014

Looks promising. Now we need to figure out the random testOverride failure :-(

@nik9000
Copy link
Contributor Author

nik9000 commented Jun 13, 2014

I'll have a look.

ruflin added a commit that referenced this pull request Jun 13, 2014
Make a test in ClientTest more consistent
@ruflin ruflin merged commit 7b34a8d into ruflin:master Jun 13, 2014
@ruflin
Copy link
Owner

ruflin commented Jun 13, 2014

Merged this one so we can check this in a new pull request. I think this problem is solved. Thx a lot.

@nik9000
Copy link
Contributor Author

nik9000 commented Jun 13, 2014

Unfortunate: I just amended and pushed a fix for the other test.

@nik9000
Copy link
Contributor Author

nik9000 commented Jun 13, 2014

Let me disentangle.

@ruflin
Copy link
Owner

ruflin commented Jun 13, 2014

Sorry :-)

@nik9000
Copy link
Contributor Author

nik9000 commented Jun 13, 2014

#635

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ClientTest->testDeleteIdsIdxStringTypeString fails about 1/3 of the time
2 participants