-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Check docs limit before indexing on primary #63273
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
Conversation
|
Pinging @elastic/es-distributed (:Distributed/Engine) |
|
run elasticsearch-ci/1 |
DaveCTurner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @dnhatn, this is great. I left a couple of minor questions.
server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java
Outdated
Show resolved
Hide resolved
server/src/internalClusterTest/java/org/elasticsearch/index/engine/MaxDocsLimitIT.java
Outdated
Show resolved
Hide resolved
|
@DaveCTurner Thanks for looking. I've addressed your comments. |
DaveCTurner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @dnhatn; implementation looks good now. I left a couple of suggestions for alternative names and an idea for a more general assertion too.
server/src/internalClusterTest/java/org/elasticsearch/index/engine/MaxDocsLimitIT.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java
Outdated
Show resolved
Hide resolved
DaveCTurner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @dnhatn
Do we consider this a bug fix? If so, do you think it should go into 7.10?
|
@DaveCTurner Thanks so much for the helpful reviews.
+1. I will backport to 7.10. |
Today indexing to a shard with 2147483519 documents will fail that shard. We should check the number of documents and reject the write requests instead. Closes elastic#51136
Today indexing to a shard with 2147483519 documents will fail that shard. We should check the number of documents and reject the write requests instead. Closes elastic#51136
Today indexing to a shard with 2147483519 documents will fail that shard. We should check the number of documents and reject the write requests instead. Closes #51136
Today indexing to a shard with 2147483519 documents will fail that shard. We should check the number of documents and reject the write requests instead. Closes #51136
This reverts commit 9015b50.
Today indexing to a shard with 2147483519 documents will fail that shard. We should check the number of documents and reject the write requests instead.
Closes #51136