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.

Sign up for GitHub

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

Jump to bottom

Allow optype CREATE for append-only indexing operations #47169

Merged
merged 8 commits into from
Sep 30, 2019
Merged

Allow optype CREATE for append-only indexing operations #47169

merged 8 commits into from
Sep 30, 2019

Conversation

ywelsch
Copy link
Contributor

@ywelsch ywelsch commented Sep 26, 2019 •
edited
Loading

Bulk requests currently do not allow adding "create" actions with auto-generated IDs. This PR allows using the optype CREATE for append-only indexing operations. This is mainly the user-facing aspect of it. Follow-ups will include:

  • checking whether these should be handled differently at the engine level
  • checking whether the default for POST index/_doc should be the optype CREATE.

@ywelsch ywelsch added >enhancement :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v8.0.0 v7.5.0 labels Sep 26, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@ywelsch
Copy link
Contributor Author

ywelsch commented Sep 26, 2019

@elasticmachine run elasticsearch-ci/2
run elasticsearch-ci/bwc
run elasticsearch-ci/default-distro

@ywelsch
Copy link
Contributor Author

ywelsch commented Sep 27, 2019

@elasticmachine run elasticsearch-ci/default-distro

@ywelsch ywelsch marked this pull request as ready for review September 27, 2019 06:23
henningandersen
henningandersen reviewed Sep 30, 2019
View reviewed changes
Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good already. I left a number of smaller comments to consider and/or address.

server/src/main/java/org/elasticsearch/action/index/IndexRequest.java Outdated Show resolved Hide resolved
server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java Show resolved Hide resolved
rest-api-spec/src/main/resources/rest-api-spec/test/bulk/10_basic.yml Show resolved Hide resolved
rest-api-spec/src/main/resources/rest-api-spec/test/bulk/11_basic_with_types.yml Show resolved Hide resolved
server/src/test/java/org/elasticsearch/document/DocumentActionsIT.java Outdated Show resolved Hide resolved
server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java Outdated Show resolved Hide resolved
server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java Outdated Show resolved Hide resolved
server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java Outdated Show resolved Hide resolved
server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java Outdated Show resolved Hide resolved
qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java Outdated Show resolved Hide resolved
ywelsch
ywelsch commented Sep 30, 2019
View reviewed changes
Copy link
Contributor Author

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the thorough review. All addressed in 3a31040

server/src/main/java/org/elasticsearch/action/index/IndexRequest.java Outdated Show resolved Hide resolved
server/src/main/java/org/elasticsearch/action/bulk/TransportBulkAction.java Show resolved Hide resolved
qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/IndexingIT.java Outdated Show resolved Hide resolved
henningandersen
henningandersen approved these changes Sep 30, 2019
View reviewed changes
Copy link
Contributor

@henningandersen henningandersen 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 think we should hold off on backporting until we have ensured that the engine properly handles op_type create requests with auto-id.

@ywelsch ywelsch merged commit c048c86 into elastic:master Sep 30, 2019
@ywelsch ywelsch mentioned this pull request Sep 30, 2019
Merged
ywelsch added a commit that referenced this pull request Oct 1, 2019
@ywelsch
Fix CloseIndexIT.testRelocatedClosedIndexIssue (#47169)
56ba1b8 
Closes #47330
ywelsch added a commit that referenced this pull request Oct 1, 2019
@ywelsch
Fix CloseIndexIT.testRelocatedClosedIndexIssue (#47169)
dd0af2e 
Closes #47330
ywelsch added a commit that referenced this pull request Oct 1, 2019
@ywelsch
Fix CloseIndexIT.testRelocatedClosedIndexIssue (#47169)
7a40cd7 
Closes #47330
ywelsch added a commit that referenced this pull request Oct 1, 2019
@ywelsch
Use standard semantics for retried auto-id requests (#47311)
9993cf3 
Adds support for handling auto-id requests with optype CREATE. Also simplifies the code
handling this by using the standard indexing path when dealing with possible retry conflicts.

Relates #47169
ywelsch added a commit to ywelsch/elasticsearch that referenced this pull request Oct 2, 2019
@ywelsch
Allow optype CREATE for append-only indexing operations (elastic#47169)
7086d1b 
Bulk requests currently do not allow adding "create" actions with auto-generated IDs.
This commit allows using the optype CREATE for append-only indexing operations. This is
mainly the user facing aspect of it.
ywelsch added a commit to ywelsch/elasticsearch that referenced this pull request Oct 2, 2019
@ywelsch
Use standard semantics for retried auto-id requests (elastic#47311)
b2b14a3 
Adds support for handling auto-id requests with optype CREATE. Also simplifies the code
handling this by using the standard indexing path when dealing with possible retry conflicts.

Relates elastic#47169
ywelsch added a commit that referenced this pull request Oct 2, 2019
@ywelsch
Allow optype CREATE for append-only indexing operations (#47169)
7b2613d 
Bulk requests currently do not allow adding "create" actions with auto-generated IDs.
This commit allows using the optype CREATE for append-only indexing operations. This is
mainly the user facing aspect of it.
ywelsch added a commit that referenced this pull request Oct 2, 2019
@ywelsch
Use standard semantics for retried auto-id requests (#47311)
8c11fe6 
Adds support for handling auto-id requests with optype CREATE. Also simplifies the code
handling this by using the standard indexing path when dealing with possible retry conflicts.

Relates #47169
@urso urso mentioned this pull request Oct 7, 2019
Merged
@Mpdreamz Mpdreamz mentioned this pull request Nov 19, 2019
Closed
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@henningandersen henningandersen henningandersen approved these changes

@bizybot bizybot Awaiting requested review from bizybot

Assignees
No one assigned
Labels
:Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >enhancement v7.5.0 v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ywelsch @elasticmachine @henningandersen @jakelandis