Skip to content

[BEAM-6052] Fixes bug in ElasticSearchIO for checkForErrors#7035

Closed
Grypse wants to merge 48 commits intoapache:masterfrom
Grypse:master
Closed

[BEAM-6052] Fixes bug in ElasticSearchIO for checkForErrors#7035
Grypse wants to merge 48 commits intoapache:masterfrom
Grypse:master

Conversation

@Grypse
Copy link
Contributor

@Grypse Grypse commented Nov 14, 2018

@timrobertson100 @echauchot
The checkFor Errors method can not parse the update response before. So i just add the judge logical for partial update, it's a minor bug fix.

The general update response looks like this below:

{
	"took": 293,
	"errors": true,
	"items": [
	{
		"update": {
			"_index": "test_kevin_2018-11",
			"_type": "kevin",
			"_id": "8d7664286c0887c637229166c7c613bc",
			"_version": 1,
			"result": "noop",
			"_shards": {
				"total": 1,
				"successful": 1,
				"failed": 0
			},
			"status": 200
		}
	},				
	{
		"update": {
			"_index": "test_kevin_2018-11",
			"_type": "kevin",
			"_id": "49952be98f4fc160f56bcdb33b1dbf7e",
			"status": 429,
			"error": {
				"type": "es_rejected_execution_exception",
				"reason": "rejected execution of org.elasticsearch.transport.TransportService$7@3f70bbe7 on EsThreadPoolExecutor[name = gjzx159-node2/write, queue capacity = 200, org.elasticsearch.common.util.concurrent.EsThreadPoolExecutor@56b9faa3[Running, pool size = 40, active threads = 40, queued tasks = 200, completed tasks = 10034174]]"
			}
		}
	}
}

So, we need to judge update for update path of jsonnode.

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

It will help us expedite review of your Pull Request if you tag someone (e.g. @username) to look at it.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- --- --- --- ---
Java Build Status Build Status Build Status Build Status Build Status Build Status Build Status Build Status
Python Build Status --- Build Status
Build Status
Build Status --- --- ---

swegner and others added 8 commits November 8, 2018 15:43
…eline options for Dataflow portability"

This is a revert of commit 3846092,
which previously reverted commit 42984a8.
This fixes 42984a8, which refactored
the Dataflow runner build scripts to share a common
buildAndPushDockerContainer task. However the task internally uses a
dockerImageContainer variable which was not shared between projects and
incorrectly duplicated: the generated name has an embedded timestamp,
which will be re-evaluated per-project.

This change refactors the projects to also share a single
dockerImageContainer property.
Copy link
Contributor

@echauchot echauchot 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 that @Grypse !

  • please add tests on the test suite for error parsing in ES2, ES5 and ES6 with partial update mode and full update mode
  • comment for later, please rebase on master instead of making a merge commit of the master to your branch.

echauchot and others added 21 commits November 14, 2018 16:28
…ces in all test utils that do index writes
Convert Top combiner to a full PTransform
fix lint error in userstate.py
Revert PR #6903 because it is breaking test error reporting
VaclavPlajt and others added 19 commits November 15, 2018 12:16
+ Enable reading table name only
+ Tests for some NexmarkUtils' methods
Merge pull request #6886: [BEAM-5906] Use dedicated BigQuery client for publishing Nexmark results
[Beam-6048] Fix for failure in beam-sdks-python:docs: Sphinx cannot find typing.G…
* [BEAM-6056] Publish gRPC 1.13.1 as a vendored dependency.

For vendored artifacts, this change
* adds license headers and properties related to our project to the pom.xml
* includes the pom.xml and pom.properties in the jar
* adds support for signing
@echauchot
Copy link
Contributor

@Grypse as I wrote above, you need to rebase your branch on master and not use merge. Right now your PR pushes commits that are already on master. Please re-make your branch:
checkout master
pull --rebase
checkout -b new-branch
and cherry pick your 2 commits on the new branch.
Please also add the tests that I was referring to above.

@Grypse
Copy link
Contributor Author

Grypse commented Nov 21, 2018 via email

@echauchot
Copy link
Contributor

OK, It's my first work with open source community. Can u teach me how to do it? Thanks!

Hi @Grypse,
I already gave you all the procedure in my comment. I adapt it a bit because you work directly on a branch called master:
git checkout -b apache-master apache/master
cherry pick (with Intellij) your 2 commits from master to apache-master
force push apache-master to Grypse:master to update your PR

@echauchot
Copy link
Contributor

closing because replaced by #7206

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.