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.

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

[Refactor] remaining ImmutableOpenMap usage to j.u.Map and remove class #7309

Merged
merged 1 commit into from
Apr 26, 2023

Conversation

nknize
Copy link
Collaborator

@nknize nknize commented Apr 26, 2023

Refactors all remaining usage of HPPC backed ImmutableOpenMap to j.u.Map. This is the last usage of the class so ImmutableOpenMap is also completely removed in favor of java.util.Map usage.

relates #5910

Refactors all remaining usage of HPPC backed ImmutableOpenMap to
j.u.Map. This is the last usage of the class so ImmutableOpenMap is
completely removed in favor of java maps.

Signed-off-by: Nicholas Walter Knize <[email protected]>
@nknize nknize added enhancement Enhancement or improvement to existing feature or request skip-changelog v2.8.0 'Issues and PRs related to version v2.8.0' labels Apr 26, 2023
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.cluster.allocation.ClusterRerouteIT.testDelayWithALargeAmountOfShards

@codecov-commenter
Copy link

Codecov Report

Merging #7309 (353c025) into main (7b8796a) will increase coverage by 0.21%.
The diff coverage is 92.19%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main    #7309      +/-   ##
============================================
+ Coverage     70.45%   70.67%   +0.21%     
- Complexity    59381    59513     +132     
============================================
  Files          4862     4859       -3     
  Lines        285489   285339     -150     
  Branches      41144    41133      -11     
============================================
+ Hits         201143   201653     +510     
+ Misses        67777    67166     -611     
+ Partials      16569    16520      -49     
Impacted Files Coverage Δ
...cluster/reroute/TransportClusterRerouteAction.java 46.05% <0.00%> (+0.59%) ⬆️
...ices/shards/TransportIndicesShardStoresAction.java 4.90% <0.00%> (ø)
...a/org/opensearch/common/io/stream/StreamInput.java 88.80% <ø> (+0.42%) ⬆️
.../org/opensearch/common/io/stream/StreamOutput.java 94.80% <ø> (+0.71%) ⬆️
.../src/main/java/org/opensearch/gateway/Gateway.java 8.33% <0.00%> (ø)
.../opensearch/rest/action/cat/RestIndicesAction.java 46.03% <0.00%> (-0.12%) ⬇️
...min/indices/shards/IndicesShardStoresResponse.java 55.55% <71.42%> (ø)
...dmin/indices/settings/get/GetSettingsResponse.java 73.17% <91.66%> (-0.64%) ⬇️
.../java/org/opensearch/index/mapper/FieldMapper.java 80.75% <94.11%> (+0.01%) ⬆️
...rch/action/admin/indices/get/GetIndexResponse.java 74.84% <100.00%> (+2.45%) ⬆️
... and 18 more

... and 469 files with indirect coverage changes

@nknize nknize changed the title [Refactor] Refactors remaining ImmutableOpenMap usage to j.u.Map [Refactor] remaining ImmutableOpenMap usage to j.u.Map and remove class Apr 26, 2023
Copy link
Member

@andrross andrross left a comment

Choose a reason for hiding this comment

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

This refactoring has caused some issues for the plugins, but since we've already started it will be good to finish.

I'm wondering if it'll be less impactful on downstream dependencies to do the backport in a single commit that combines all the ImmutableOpenMap removal changes.

@andrross andrross merged commit d984f50 into opensearch-project:main Apr 26, 2023
@nknize
Copy link
Collaborator Author

nknize commented Apr 27, 2023

I'm wondering if it'll be less impactful on downstream dependencies to do the backport in a single commit that combines all the ImmutableOpenMap removal changes.

I'm not opposed that. Or I can take care of all of these backports pretty quickly myself.

joshpalis added a commit to joshpalis/anomaly-detection that referenced this pull request Apr 27, 2023
owaiskazi19 pushed a commit to opensearch-project/anomaly-detection that referenced this pull request Apr 27, 2023
…y and multi-entity detectors (#880)

* Initial historical analysis commit. Registers ForwardADTaskTransportAction, ADBatchAnomalyResultTransportAction, replaces Internal Aggregation classes with corresponding Parsed Aggregation class. removes hashring from historical analysis workflow

Signed-off-by: Joshua Palis <[email protected]>

* Reverts changes to AnomalyResultBulkIndexHandler and uses the SDKRestClient to bulk index anomaly results, replaces transport service calls with corresponding client execute calls. Adds check for request content for job details registration, this prevents historical task requests from triggering job details registration

Signed-off-by: Joshua Palis <[email protected]>

* Adds another check for job details registration for start path only, adds transport action for stop historical detector

Signed-off-by: Joshua Palis <[email protected]>

* Fixing import error, test classes

Signed-off-by: Joshua Palis <[email protected]>

* spotlessApply

Signed-off-by: Joshua Palis <[email protected]>

* Commenting out parse bucket tests since ParsedAggregation classes have no constructor to access via reflection

Signed-off-by: Joshua Palis <[email protected]>

* Addressing PR comments

Signed-off-by: Joshua Palis <[email protected]>

* replaceing .valuesIt() with .values().iterator() due tohttps://github.com/opensearch-project/OpenSearch/pull/7309

Signed-off-by: Joshua Palis <[email protected]>

* removing ImmutableOpenMap

Signed-off-by: Joshua Palis <[email protected]>

* Reverting dataGeneration script changes

Signed-off-by: Joshua Palis <[email protected]>

* fixing iterator

Signed-off-by: Joshua Palis <[email protected]>

* Fixing data nodes iterator

Signed-off-by: Joshua Palis <[email protected]>

* removing value from iterator

Signed-off-by: Joshua Palis <[email protected]>

---------

Signed-off-by: Joshua Palis <[email protected]>
austintlee pushed a commit to austintlee/OpenSearch that referenced this pull request Apr 28, 2023
…nsearch-project#7309)

Refactors all remaining usage of HPPC backed ImmutableOpenMap to
j.u.Map. This is the last usage of the class so ImmutableOpenMap is
completely removed in favor of java maps.

Signed-off-by: Nicholas Walter Knize <[email protected]>
@soosinha
Copy link
Member

Due to this refactoring, we have seen improvement in heap memory usage. The MultiFields object in the FieldMapper class was modified to use Collections.UnmodifiableMap instead of ImmutableOpenMap. This reduced the retained memory usage of a multiFields object from 200 bytes to 64 bytes. When multiplying this gain by the number of fields and the number of indices in the cluster, we are observing considerable reduction in heap memory usage.

@nknize nknize added the backport 2.x Backport to 2.x branch label Jun 27, 2023
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-7309-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d984f5028e77e281ed08386d43de4e64e06f2bf8
# Push it to GitHub
git push --set-upstream origin backport/backport-7309-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-7309-to-2.x.

nknize added a commit to nknize/OpenSearch that referenced this pull request Jun 28, 2023
…nsearch-project#7309)

Refactors all remaining usage of HPPC backed ImmutableOpenMap to
j.u.Map. This is the last usage of the class so ImmutableOpenMap is
completely removed in favor of java maps.

Signed-off-by: Nicholas Walter Knize <[email protected]>
(cherry picked from commit d984f50)
@nknize nknize added v2.9.0 'Issues and PRs related to version v2.9.0' and removed v2.8.0 'Issues and PRs related to version v2.8.0' labels Jun 28, 2023
nknize added a commit to nknize/OpenSearch that referenced this pull request Jun 30, 2023
…nsearch-project#7309)

Refactors all remaining usage of HPPC backed ImmutableOpenMap to
j.u.Map. This is the last usage of the class so ImmutableOpenMap is
completely removed in favor of java maps.

Signed-off-by: Nicholas Walter Knize <[email protected]>
(cherry picked from commit d984f50)
nknize added a commit that referenced this pull request Jun 30, 2023
…) (#8316)

Refactors all remaining usage of HPPC backed ImmutableOpenMap to
j.u.Map. This is the last usage of the class so ImmutableOpenMap is
completely removed in favor of java maps.

Signed-off-by: Nicholas Walter Knize <[email protected]>
(cherry picked from commit d984f50)
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…nsearch-project#7309)

Refactors all remaining usage of HPPC backed ImmutableOpenMap to
j.u.Map. This is the last usage of the class so ImmutableOpenMap is
completely removed in favor of java maps.

Signed-off-by: Nicholas Walter Knize <[email protected]>
Signed-off-by: Shivansh Arora <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch enhancement Enhancement or improvement to existing feature or request skip-changelog v2.9.0 'Issues and PRs related to version v2.9.0'
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants