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

[ML] Prevent stack overflow while copying ML jobs and datafeeds #36370

Merged
merged 2 commits into from Dec 7, 2018

Conversation

Projects
None yet
3 participants
@dimitris-athanasiou
Contributor

dimitris-athanasiou commented Dec 7, 2018

ML jobs and datafeeds wrap collections into their unmodifiable
equivalents in their constructor. However, the copying builder
does not make a copy of some of those collections resulting
in wrapping those again and again. This can eventually result
to stack overflow.

This commit addressed this issue by copying the collections in
question in the copying builder constructor.

Closes #36360

[ML] Prevent stack overflow while copying ML jobs and datafeeds
ML jobs and datafeeds wrap collections into their unmodifiable
equivalents in their constructor. However, the copying builder
does not make a copy of some of those collections resulting
in wrapping those again and again. This can eventually result
to stack overflow.

This commit addressed this issue by copying the collections in
question in the copying builder constructor.

Closes #36360
@elasticmachine

This comment has been minimized.

elasticmachine commented Dec 7, 2018

@dimitris-athanasiou dimitris-athanasiou added v6.5.4 and removed v6.5.3 labels Dec 7, 2018

this.indices = new ArrayList<>(config.indices);
this.types = new ArrayList<>(config.types);
this.query = config.query == null ? null : new HashMap<>(config.query);
this.aggregations = config.aggregations == null ? null : new HashMap<>(config.aggregations);

This comment has been minimized.

@droberts195

droberts195 Dec 7, 2018

Contributor

I think query and aggregations should use LinkedHashMap to retain the original order.

@droberts195

LGTM

@dimitris-athanasiou dimitris-athanasiou merged commit 7870ae8 into elastic:master Dec 7, 2018

7 checks passed

CLA Commit author is a member of Elasticsearch
Details
elasticsearch-ci-1 Build finished.
Details
elasticsearch-ci-2 Build finished.
Details
elasticsearch-ci/default-distro Build finished.
Details
elasticsearch-ci/docbldesx Build finished.
Details
elasticsearch-ci/oss-distro-docs Build finished.
Details
elasticsearch-ci/packaging-sample Build finished.
Details

@dimitris-athanasiou dimitris-athanasiou deleted the dimitris-athanasiou:prevent-stack-overflow-while-copying-ml-jobs-and-datafeeds branch Dec 7, 2018

dimitris-athanasiou added a commit that referenced this pull request Dec 7, 2018

[ML] Prevent stack overflow while copying ML jobs and datafeeds (#36370)
ML jobs and datafeeds wrap collections into their unmodifiable
equivalents in their constructor. However, the copying builder
does not make a copy of some of those collections resulting
in wrapping those again and again. This can eventually result
to stack overflow.

This commit addressed this issue by copying the collections in
question in the copying builder constructor.

Closes #36360

dimitris-athanasiou added a commit that referenced this pull request Dec 7, 2018

[ML] Prevent stack overflow while copying ML jobs and datafeeds (#36370)
ML jobs and datafeeds wrap collections into their unmodifiable
equivalents in their constructor. However, the copying builder
does not make a copy of some of those collections resulting
in wrapping those again and again. This can eventually result
to stack overflow.

This commit addressed this issue by copying the collections in
question in the copying builder constructor.

Closes #36360

jasontedor added a commit to liketic/elasticsearch that referenced this pull request Dec 9, 2018

Merge remote-tracking branch 'elastic/6.x' into pr/28862
* elastic/6.x: (37 commits)
  [HLRC] Added support for Follow Stats API (elastic#36253)
  Exposed engine must have all ops below gcp during rollback (elastic#36159)
  TEST: Always enable soft-deletes in ShardChangesTests
  Use delCount of SegmentInfos to calculate numDocs (elastic#36323)
  Add soft-deletes upgrade tests (elastic#36286)
  Remove LocalCheckpointTracker#resetCheckpoint (elastic#34667)
  Option to use endpoints starting with _security (elastic#36379)
  [CCR] Restructured QA modules (elastic#36404)
  RestClient: on retry timeout add root exception (elastic#25576)
  [HLRC] Add support for put privileges API (elastic#35679)
  HLRC: Add rollup search (elastic#36334)
  Explicitly recommend to forceMerge before freezing (elastic#36376)
  Rename internal repository actions to be internal (elastic#36377)
  Core: Remove parseDefaulting from DateFormatter (elastic#36386)
  [ML] Prevent stack overflow while copying ML jobs and datafeeds (elastic#36370)
  Docs: Fix Jackson reference (elastic#36366)
  [ILM] Fix issue where index may not yet be in 'hot' phase (elastic#35716)
  Undeprecate /_watcher endpoints (elastic#36269)
  Docs: Fix typo in bool query (elastic#36350)
  HLRC: Add delete template API (elastic#36320)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment