Check UTF16 string size before converting to String to avoid OOME #7963

imRishN · 2023-06-08T10:31:43Z

Description

Node can drop out for OutOfMemory errors other than the VM OutOfMemory which comes from the implementation limits of StringUTF16.

java.lang.OutOfMemoryError: UTF16 String size is 1089861360, should be less than 1073741823

This PR attempts to pre check the size and throw an exception proactively before trying a conversion.

Related Issues

Resolves #1651

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

github-actions · 2023-06-08T10:45:18Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/17117/
CommitID: 4c52178
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2023-06-08T10:57:45Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/17116/
CommitID: d27cd97
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

server/src/main/java/org/opensearch/common/bytes/AbstractBytesReference.java

github-actions · 2023-06-08T19:07:40Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.search.backpressure.SearchBackpressureIT.testSearchShardTaskCancellationWithHighCpu

URL: https://build.ci.opensearch.org/job/gradle-check/17161/
CommitID: 242c3ce
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

codecov · 2023-06-08T19:12:21Z

Codecov Report

Merging #7963 (be82717) into main (976048d) will increase coverage by 0.03%.
The diff coverage is 89.47%.

@@             Coverage Diff              @@
##               main    #7963      +/-   ##
============================================
+ Coverage     70.87%   70.91%   +0.03%     
- Complexity    56904    56908       +4     
============================================
  Files          4756     4757       +1     
  Lines        269064   269106      +42     
  Branches      39392    39396       +4     
============================================
+ Hits         190710   190831     +121     
+ Misses        62285    62151     -134     
- Partials      16069    16124      +55

Impacted Files	Coverage Δ
...nsearch/action/search/ArraySearchPhaseResults.java	`90.90% <ø> (ø)`
...java/org/opensearch/action/search/SearchPhase.java	`80.00% <0.00%> (-20.00%)`	⬇️
...g/opensearch/action/search/SearchPhaseContext.java	`66.66% <ø> (ø)`
...g/opensearch/action/search/SearchPhaseResults.java	`71.42% <ø> (ø)`
...a/org/opensearch/plugins/SearchPipelinePlugin.java	`0.00% <0.00%> (ø)`
...earch/action/search/AbstractSearchAsyncAction.java	`77.94% <75.00%> (-1.00%)`	⬇️
.../java/org/opensearch/search/pipeline/Pipeline.java	`86.04% <75.00%> (-1.80%)`	⬇️
...pensearch/common/bytes/AbstractBytesReference.java	`84.66% <100.00%> (+0.58%)`	⬆️
...ch/action/search/CanMatchPreFilterSearchPhase.java	`93.44% <100.00%> (-3.23%)`	⬇️
...va/org/opensearch/action/search/DfsQueryPhase.java	`90.24% <100.00%> (ø)`
... and 10 more

... and 470 files with indirect coverage changes

BhumikaSaini-Amazon

Thanks for adding this!

Few minor comments and we should add a unit test

CHANGELOG.md

server/src/main/java/org/opensearch/common/bytes/AbstractBytesReference.java

github-actions · 2023-06-09T07:34:02Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.search.backpressure.SearchBackpressureIT.testSearchTaskCancellationWithHighCpu

URL: https://build.ci.opensearch.org/job/gradle-check/17203/
CommitID: 22cb692
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

github-actions · 2023-06-27T13:01:29Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/18452/
CommitID: 532e198
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2023-06-27T13:07:21Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/18453/
CommitID: 8976718
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2023-06-27T14:04:44Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/18455/
CommitID: de291c6
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2023-06-27T14:11:59Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/18456/
CommitID: 8d43d21
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

Signed-off-by: Rishab Nahata <[email protected]>

github-actions · 2023-06-27T18:16:36Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testNodeDropWithOngoingReplication

URL: https://build.ci.opensearch.org/job/gradle-check/18487/
CommitID: 6be9337
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

libs/core/src/main/java/org/opensearch/common/bytes/AbstractBytesReference.java

Signed-off-by: Rishab Nahata <[email protected]>

github-actions · 2023-06-28T05:45:56Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testNodeDropWithOngoingReplication
      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testDropPrimaryDuringReplication
      1 org.opensearch.indices.replication.SegmentReplicationIT.testScrollCreatedOnReplica

URL: https://build.ci.opensearch.org/job/gradle-check/18539/
CommitID: cea8789
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

dblock · 2023-06-28T22:44:46Z

@Bukhtawar @reta ya'll good with this as is now?

Signed-off-by: Daniel (dB.) Doubrovkine <[email protected]>

github-actions · 2023-06-28T23:20:22Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/18622/
CommitID: 0605f3f
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2023-06-29T00:09:39Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testNodeDropWithOngoingReplication
      1 org.opensearch.indices.replication.SegmentReplicationIT.testScrollCreatedOnReplica

URL: https://build.ci.opensearch.org/job/gradle-check/18629/
CommitID: 0605f3f
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Signed-off-by: Rishab Nahata <[email protected]>

github-actions · 2023-06-29T03:53:59Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testNodeDropWithOngoingReplication

URL: https://build.ci.opensearch.org/job/gradle-check/18648/
CommitID: be82717
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

opensearch-trigger-bot · 2023-06-29T05:23:00Z

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-7963-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 544b1cac97a848479f2355e11e1a2c18b40e8201
# Push it to GitHub
git push --set-upstream origin backport/backport-7963-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-7963-to-2.x.

…ensearch-project#7963) * Check UTF16 string size before converting to string to avoid OOME Signed-off-by: Rishab Nahata <[email protected]>

reta · 2023-06-29T11:43:39Z

~~@imRishN could you please submit manual backport for 2.x? thank you~~ is there already: #8344

) (#8344) * Check UTF16 string size before converting to string to avoid OOME Signed-off-by: Rishab Nahata <[email protected]>

…ensearch-project#7963) * Check UTF16 string size before converting to string to avoid OOME Signed-off-by: Rishab Nahata <[email protected]>

…ensearch-project#7963) * Check UTF16 string size before converting to string to avoid OOME Signed-off-by: Rishab Nahata <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>

imRishN requested review from reta, anasalkouz, andrross, Bukhtawar, CEHENKLE, dblock, gbbafna, setiah, kartg, kotwanikunal, mch2, nknize, owaiskazi19, Rishikesh1159, ryanbogan, saratvemulapalli, shwetathareja, dreamer-89, tlfeng, VachaShah and dbwiddis as code owners June 8, 2023 10:31

reta reviewed Jun 8, 2023

View reviewed changes

server/src/main/java/org/opensearch/common/bytes/AbstractBytesReference.java Outdated Show resolved Hide resolved

reta reviewed Jun 8, 2023

View reviewed changes

server/src/main/java/org/opensearch/common/bytes/AbstractBytesReference.java Outdated Show resolved Hide resolved

BhumikaSaini-Amazon suggested changes Jun 9, 2023

View reviewed changes

CHANGELOG.md Show resolved Hide resolved

server/src/main/java/org/opensearch/common/bytes/AbstractBytesReference.java Outdated Show resolved Hide resolved

server/src/main/java/org/opensearch/common/bytes/AbstractBytesReference.java Outdated Show resolved Hide resolved

Bukhtawar reviewed Jun 9, 2023

View reviewed changes

server/src/main/java/org/opensearch/common/bytes/AbstractBytesReference.java Outdated Show resolved Hide resolved

imRishN requested a review from sachinpkale as a code owner June 27, 2023 12:55

imRishN requested a review from reta June 27, 2023 13:41

Check UTF16 string size before converting to string to avoid OOME

6be9337

Signed-off-by: Rishab Nahata <[email protected]>

imRishN force-pushed the string-limit branch from 8d43d21 to 6be9337 Compare June 27, 2023 17:41

Bukhtawar reviewed Jun 27, 2023

View reviewed changes

libs/core/src/main/java/org/opensearch/common/bytes/AbstractBytesReference.java Outdated Show resolved Hide resolved

Bukhtawar reviewed Jun 27, 2023

View reviewed changes

libs/core/src/main/java/org/opensearch/common/bytes/AbstractBytesReference.java Outdated Show resolved Hide resolved

Minor fix

cea8789

Signed-off-by: Rishab Nahata <[email protected]>

Merge branch 'main' into string-limit

0605f3f

Signed-off-by: Daniel (dB.) Doubrovkine <[email protected]>

reta approved these changes Jun 28, 2023

View reviewed changes

Merge branch 'main' into string-limit

be82717

Signed-off-by: Rishab Nahata <[email protected]>

Bukhtawar approved these changes Jun 29, 2023

View reviewed changes

Bukhtawar merged commit 544b1ca into opensearch-project:main Jun 29, 2023

imRishN added the backport 2.x Backport to 2.x branch label Jun 29, 2023

reta mentioned this pull request Jun 29, 2023

[Backport -2.x] Check UTF16 string size before converting to String to avoid OOME #8344

Merged

6 tasks

reta pushed a commit that referenced this pull request Jun 29, 2023

Check UTF16 string size before converting to String to avoid OOME (#7963

9ee8fe6

) (#8344) * Check UTF16 string size before converting to string to avoid OOME Signed-off-by: Rishab Nahata <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check UTF16 string size before converting to String to avoid OOME #7963

Check UTF16 string size before converting to String to avoid OOME #7963

imRishN commented Jun 8, 2023 •

edited

Loading

github-actions bot commented Jun 8, 2023

github-actions bot commented Jun 8, 2023

github-actions bot commented Jun 8, 2023

codecov bot commented Jun 8, 2023 •

edited

Loading

BhumikaSaini-Amazon left a comment

github-actions bot commented Jun 9, 2023

github-actions bot commented Jun 27, 2023

github-actions bot commented Jun 27, 2023

github-actions bot commented Jun 27, 2023

github-actions bot commented Jun 27, 2023

github-actions bot commented Jun 27, 2023

github-actions bot commented Jun 28, 2023

dblock commented Jun 28, 2023

github-actions bot commented Jun 28, 2023

github-actions bot commented Jun 29, 2023

github-actions bot commented Jun 29, 2023

opensearch-trigger-bot bot commented Jun 29, 2023

reta commented Jun 29, 2023 •

edited

Loading

Check UTF16 string size before converting to String to avoid OOME #7963

Check UTF16 string size before converting to String to avoid OOME #7963

Conversation

imRishN commented Jun 8, 2023 • edited Loading

Description

Related Issues

Check List

github-actions bot commented Jun 8, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jun 8, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jun 8, 2023

Gradle Check (Jenkins) Run Completed with:

codecov bot commented Jun 8, 2023 • edited Loading

Codecov Report

BhumikaSaini-Amazon left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 9, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jun 27, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jun 27, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jun 27, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jun 27, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jun 27, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jun 28, 2023

Gradle Check (Jenkins) Run Completed with:

dblock commented Jun 28, 2023

github-actions bot commented Jun 28, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jun 29, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jun 29, 2023

Gradle Check (Jenkins) Run Completed with:

opensearch-trigger-bot bot commented Jun 29, 2023

reta commented Jun 29, 2023 • edited Loading

imRishN commented Jun 8, 2023 •

edited

Loading

codecov bot commented Jun 8, 2023 •

edited

Loading

reta commented Jun 29, 2023 •

edited

Loading