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

Avoid doing redundant work when checking for self references. #26927

Merged
merged 1 commit into from
Jan 15, 2018
Merged

Avoid doing redundant work when checking for self references. #26927

merged 1 commit into from
Jan 15, 2018

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Oct 9, 2017

Currently we test all maps, arrays or iterables. However, in the case that maps
contain sub maps for instance, we will test the sub maps again even though the
work has already been done for the top-level map.

Relates #26907

rjernst
rjernst approved these changes Oct 9, 2017
View reviewed changes
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

@jgq2008303393
Copy link

should IndexRequest.source(IndexRequest.java:375) be updated?

elasticsearch[1505480279000001809][bulk][T#19]" #255 daemon prio=5 os_prio=0 tid=0x00007fdd6c012800 nid=0xa86 runnable [0x00007fe823238000]
   java.lang.Thread.State: RUNNABLE
        at java.lang.System.identityHashCode(Native Method)
        at java.util.IdentityHashMap.hash(IdentityHashMap.java:294)
        at java.util.IdentityHashMap.put(IdentityHashMap.java:425)
        at java.util.Collections$SetFromMap.add(Collections.java:5461)
        at org.elasticsearch.common.xcontent.XContentBuilder.ensureNoSelfReferences(XContentBuilder.java:1088)
        at org.elasticsearch.common.xcontent.XContentBuilder.ensureNoSelfReferences(XContentBuilder.java:1071)
        at org.elasticsearch.common.xcontent.XContentBuilder.map(XContentBuilder.java:872)
        at org.elasticsearch.action.index.IndexRequest.source(IndexRequest.java:375)
        at org.elasticsearch.action.index.IndexRequest.source(IndexRequest.java:364)
        at org.elasticsearch.ingest.PipelineExecutionService.innerExecute(PipelineExecutionService.java:179)
        at org.elasticsearch.ingest.PipelineExecutionService.access$000(PipelineExecutionService.java:41)
        at org.elasticsearch.ingest.PipelineExecutionService$2.doRun(PipelineExecutionService.java:88)
        at org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:638)
        at org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:37)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
        at java.lang.Thread.run(Thread.java:745)

@jpountz
Avoid doing redundant work when checking for self references.
f2e3d42 
Currently we test all maps, arrays or iterables. However, in the case that maps
contain sub maps for instance, we will test the sub maps again even though the
work has already been done for the top-level map.

Relates elastic#26907
@jpountz jpountz force-pushed the fix/duplicate_ensure_no_self_references branch from 8b5dde2 to f2e3d42 Compare January 15, 2018 09:55
@jpountz
Copy link
Contributor Author

jpountz commented Jan 15, 2018

@jgq2008303393 It will automatically get the change since it uses XContentBuilder.map.

@jpountz jpountz merged commit 0a92e43 into elastic:master Jan 15, 2018
@jpountz jpountz deleted the fix/duplicate_ensure_no_self_references branch January 15, 2018 17:36
jpountz added a commit that referenced this pull request Jan 15, 2018
@jpountz
Avoid doing redundant work when checking for self references. (#26927)
9854255 
Currently we test all maps, arrays or iterables. However, in the case that maps
contain sub maps for instance, we will test the sub maps again even though the
work has already been done for the top-level map.

Relates #26907
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jan 15, 2018
@jasontedor
Merge branch 'master' into compile-with-jdk-9
30dbcb4 
* master: (21 commits)
  [GEO] Add WKT Support to GeoBoundingBoxQueryBuilder
  Painless: Add whitelist extensions (elastic#28161)
  Fix daitch_mokotoff phonetic filter to use the dedicated Lucene filter (elastic#28225)
  Avoid doing redundant work when checking for self references. (elastic#26927)
  Fix casts in HotThreads. (elastic#27578)
  Ignore the `-snapshot` suffix when comparing the Lucene version in the build and the docs. (elastic#27927)
  Allow update of `eager_global_ordinals` on `_parent`. (elastic#28014)
  Fix NPE on composite aggregation with sub-aggregations that need scores (elastic#28129)
  `MockTcpTransport` to connect asynchronously (elastic#28203)
  Fix synonym phrase query expansion for cross_fields parsing (elastic#28045)
  Introduce elasticsearch-core jar (elastic#28191)
  elastic#28218: Update the Lucene version for 6.2.0 after backport
  upgrade to lucene 7.2.1 (elastic#28218)
  [Docs] Fix an error in painless-types.asciidoc (elastic#28221)
  Adds metadata to rewritten aggregations (elastic#28185)
  Update version of TaskInfo header serialization after backport
  TEST: Tightens file-based condition in peer-recovery
  Correct backport replica rollback to 6.2 (elastic#28181)
  Backport replica rollback to 6.2 (elastic#28181)
  Rename deleteLocalTranslog to createNewTranslog
  ...
@s1monw
Copy link
Contributor

s1monw commented Jan 16, 2018

@jpountz can you add version labels to this PR please

@jpountz
Copy link
Contributor Author

jpountz commented Jan 16, 2018

@simonw done

@s1monw
Copy link
Contributor

s1monw commented Jan 16, 2018

thx

martijnvg added a commit that referenced this pull request Jan 16, 2018
@martijnvg
Merge remote-tracking branch 'es/6.x' into ccr-6.x
76809e7 
* es/6.x: (31 commits)
  Fix eclipse build. (#28236)
  Never return null from Strings.tokenizeToStringArray (#28224)
  Fallback to TransportMasterNodeAction for cluster health retries (#28195)
  [Docs] Changes to ingest.asciidoc (#28212)
  TEST: Update logging for testAckedIndexing
  [GEO] Deprecate field parameter in GeoBoundingBoxQueryBuilder
  [GEO] Add WKT Support to GeoBoundingBoxQueryBuilder
  Avoid doing redundant work when checking for self references. (#26927)
  Fix casts in HotThreads. (#27578)
  Ignore the `-snapshot` suffix when comparing the Lucene version in the build and the docs. (#27927)
  Allow update of `eager_global_ordinals` on `_parent`. (#28014)
  Painless: Add whitelist extensions (#28161)
  Fix daitch_mokotoff phonetic filter to use the dedicated Lucene filter (#28225)
  Fix NPE on composite aggregation with sub-aggregations that need scores (#28129)
  #28045 restore removed import after backport
  Fix synonym phrase query expansion for cross_fields parsing (#28045)
  Introduce elasticsearch-core jar (#28191)
  upgrade to lucene 7.2.1 (#28218)
  [Docs] Fix an error in painless-types.asciidoc (#28221)
  Consistent updates of IndexShardSnapshotStatus (#28130)
  ...
martijnvg added a commit that referenced this pull request Jan 16, 2018
@martijnvg
Merge remote-tracking branch 'es/master' into ccr
f2093e7 
* es/master: (30 commits)
  [Docs] Fix Java Api index administration usage (#28133)
  Fix eclipse build. (#28236)
  Never return null from Strings.tokenizeToStringArray (#28224)
  Fallback to TransportMasterNodeAction for cluster health retries (#28195)
  [Docs] Changes to ingest.asciidoc (#28212)
  TEST: Update logging for testAckedIndexing
  [GEO] Add WKT Support to GeoBoundingBoxQueryBuilder
  Painless: Add whitelist extensions (#28161)
  Fix daitch_mokotoff phonetic filter to use the dedicated Lucene filter (#28225)
  Avoid doing redundant work when checking for self references. (#26927)
  Fix casts in HotThreads. (#27578)
  Ignore the `-snapshot` suffix when comparing the Lucene version in the build and the docs. (#27927)
  Allow update of `eager_global_ordinals` on `_parent`. (#28014)
  Fix NPE on composite aggregation with sub-aggregations that need scores (#28129)
  `MockTcpTransport` to connect asynchronously (#28203)
  Fix synonym phrase query expansion for cross_fields parsing (#28045)
  Introduce elasticsearch-core jar (#28191)
  #28218: Update the Lucene version for 6.2.0 after backport
  upgrade to lucene 7.2.1 (#28218)
  [Docs] Fix an error in painless-types.asciidoc (#28221)
  ...
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jan 16, 2018
@jasontedor
Merge branch 'master' into mrjar
57972df 
* master: (35 commits)
  Move the multi-get response tests to server
  Require JDK 9 for compilation (elastic#28071)
  Revert "[Docs] Fix Java Api index administration usage (elastic#28133)"
  Revert "[Docs] Fix base directory to include for put_mapping.asciidoc"
  Added multi get api to the high level rest client.
  [Docs] Clarify numeric datatype ranges (elastic#28240)
  [Docs] Fix base directory to include for put_mapping.asciidoc
  Open engine should keep only starting commit (elastic#28228)
  [Docs] Fix Java Api index administration usage (elastic#28133)
  Fix eclipse build. (elastic#28236)
  Never return null from Strings.tokenizeToStringArray (elastic#28224)
  Fallback to TransportMasterNodeAction for cluster health retries (elastic#28195)
  [Docs] Changes to ingest.asciidoc (elastic#28212)
  TEST: Update logging for testAckedIndexing
  [GEO] Add WKT Support to GeoBoundingBoxQueryBuilder
  Painless: Add whitelist extensions (elastic#28161)
  Fix daitch_mokotoff phonetic filter to use the dedicated Lucene filter (elastic#28225)
  Avoid doing redundant work when checking for self references. (elastic#26927)
  Fix casts in HotThreads. (elastic#27578)
  Ignore the `-snapshot` suffix when comparing the Lucene version in the build and the docs. (elastic#27927)
  ...
@jpountz jpountz added the :Core/Infra/Core Core issues without another label label Jan 29, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rjernst rjernst rjernst approved these changes

Assignees
No one assigned
Labels
:Core/Infra/Core Core issues without another label >enhancement v6.2.0 v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jpountz @jgq2008303393 @s1monw @elasticmachine @rjernst @colings86