Skip to content

[Docs] Document Bulk Processor for Java High Level REST Client#25572

Merged
tlrx merged 2 commits intoelastic:masterfrom
tlrx:bulk-processor-doc
Jul 6, 2017
Merged

[Docs] Document Bulk Processor for Java High Level REST Client#25572
tlrx merged 2 commits intoelastic:masterfrom
tlrx:bulk-processor-doc

Conversation

@tlrx
Copy link
Copy Markdown
Member

@tlrx tlrx commented Jul 6, 2017

This PR adds documentation for the Bulk Processor usage with the Java High Level REST Client.

Copy link
Copy Markdown
Contributor

@javanna javanna left a comment

Choose a reason for hiding this comment

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

a utility class that allows index/update/delete operations to be
transparently executed as they are added to the processor.

In order to execute the requests, the `BulkProcessor` requires 3 things:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/things/components ?

In order to execute the requests, the `BulkProcessor` requires 3 things:

`RestHighLevelClient`:: This client is used to execute the `BulkRequest`s
and to retrieve the `BulkResponse`s
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't see why but BulkRequest and BulkResponse are not visualized properly

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we want to mention that the bulkAsync method will be called internally (as we provide it when creating the processor)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't see why but BulkRequest and BulkResponse are not visualized properly

I removed the s at the end of those

do we want to mention that the bulkAsync method will be called internally (as we provide it when creating the processor)

I mentioned the method later, in the sample code where the processor is instantiated. I think we're good?

<3> Called if the `BulkRequest` failed, this method allows to know
the failure

When all requests have been added to the `BulkProcessor` and this one is
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Once all requests have been added to the BulkProcessor, its instance needs to be closed using of the two available close methods.

--------------------------------------------------

Both methods flush the requests added to the processor before closing the processor
and also forbids any new request to be added to it.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

forbid (no s at the end)

@tlrx
Copy link
Copy Markdown
Member Author

tlrx commented Jul 6, 2017

Do we also want to borrow this paragraph: https://www.elastic.co/guide/en/elasticsearch/client/java-api/current/java-docs-bulk-processor.html#java-docs-bulk-processor-tests ?

I tend to think we should not add it in the high level docs. This looks like a general advice about refreshing indices before searching them in tests and it applies to all index/bulk/update/delete requests too. Also I don't really like that it indicates a large numbers of actions and 0 concurrent requests where it could be parallelized also in tests.

Copy link
Copy Markdown
Contributor

@javanna javanna left a comment

Choose a reason for hiding this comment

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

LGTM as-is thanks @tlrx

@tlrx tlrx merged commit d9bc0f4 into elastic:master Jul 6, 2017
@tlrx tlrx deleted the bulk-processor-doc branch July 6, 2017 15:10
@tlrx
Copy link
Copy Markdown
Member Author

tlrx commented Jul 6, 2017

Thanks @javanna

jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jul 6, 2017
* master:
  Fix cluster health wait conditions in rolling restart tests
  Add cluster name validation to RemoteClusterConnection (elastic#25568)
  Removes deprecated usage of the FieldStats API in a test that verifies sequence number data in Lucene commit points.  Instead, the test retrieves the _seq_no value from the commit point directly and converts it to a Long value.
  [Tests] Check output of SuggestionBuilder#build method (elastic#25549)
  [Docs] Document Bulk Processor for Java High Level REST Client (elastic#25572)
  Update REST client deps license and notice files (elastic#25573)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants