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

Bump jackson and jackson_databind from 2.15.2 to 2.16.0 #11273

Merged
merged 3 commits into from
Nov 20, 2023

Conversation

cwperks
Copy link
Member

@cwperks cwperks commented Nov 20, 2023

Description

Bumps jackson and jackson_databind from 2.15.2 to 2.16.0

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)
  • Public documentation issue/PR created

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.

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Copy link
Contributor

github-actions bot commented Nov 20, 2023

Compatibility status:

Checks if related components are compatible with change c73b6bc

Incompatible components

Incompatible components: [https://github.com/opensearch-project/performance-analyzer.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/alerting.git]

@reta
Copy link
Collaborator

reta commented Nov 20, 2023

Two changes that might have an impact (potentially):

Maximum Output nesting depth

Implementation of jackson-core#1055 sets upper limit on maximum output nesting (Objects, Arrays) when generating output (writing JSON etc). Default limit is:

1000 levels

Maximum Property name length

Implementation of jackson-core#1047 sets maximum length of allowed Property names when parsing input. Default limit is:

50,000 units (bytes or chars, depending on input source)

Copy link
Contributor

❌ Gradle check result for d969ce3: FAILURE

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?

Copy link
Contributor

❌ Gradle check result for 1ededea: FAILURE

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?

@cwperks
Copy link
Member Author

cwperks commented Nov 20, 2023

> Task :server:internalClusterTest

Tests with failures:
 - org.opensearch.operateAllIndices.DestructiveOperationsIT.testOpenIndexDefaultBehaviour

@cwperks
Copy link
Member Author

cwperks commented Nov 20, 2023

I opened an issue to track the failing test: #11275

@reta Those new jackson values seem really large and accommodative of any realistic use-case.

@reta
Copy link
Collaborator

reta commented Nov 20, 2023

@reta Those new jackson values seem really large and accommodative of any realistic use-case.

Correct, BUT we have own settings for that, index.mapping.depth.limit and index.mapping.field_name_length.limit (inherited from Elasticsearch [1]), which potentially may be set to exceed those defaults.

[1] https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-settings-limit.html

Copy link
Contributor

❕ Gradle check result for c73b6bc: UNSTABLE

  • TEST FAILURES:
      1 org.opensearch.test.rest.ClientYamlTestSuiteIT.test {p0=indices.get/10_basic/Should return test_index_2 if expand_wildcards=open}
      1 org.opensearch.test.rest.ClientYamlTestSuiteIT.classMethod
      1 org.opensearch.smoketest.SmokeTestMultiNodeClientYamlTestSuiteIT.test {yaml=indices.rollover/40_mapping/Typeless mapping}
      1 org.opensearch.smoketest.SmokeTestMultiNodeClientYamlTestSuiteIT.classMethod
      1 org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=search.aggregation/20_terms/string profiler via global ordinals}
      1 org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.classMethod

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Copy link

codecov bot commented Nov 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (60c46f3) 71.28% compared to head (c73b6bc) 71.32%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #11273      +/-   ##
============================================
+ Coverage     71.28%   71.32%   +0.04%     
- Complexity    58942    58961      +19     
============================================
  Files          4890     4890              
  Lines        277403   277403              
  Branches      40303    40303              
============================================
+ Hits         197738   197849     +111     
+ Misses        63238    63147      -91     
+ Partials      16427    16407      -20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cwperks
Copy link
Member Author

cwperks commented Nov 20, 2023

The defaults in OpenSearch are:

public static final Setting<Long> INDEX_MAPPING_DEPTH_LIMIT_SETTING = Setting.longSetting(
    "index.mapping.depth.limit",
    20L,
    1,
    Property.Dynamic,
    Property.IndexScope
);
public static final Setting<Long> INDEX_MAPPING_FIELD_NAME_LENGTH_LIMIT_SETTING = Setting.longSetting(
    "index.mapping.field_name_length.limit",
    Long.MAX_VALUE,
    1L,
    Property.Dynamic,
    Property.IndexScope
);

Should I see if we can leverage jackson to enforce these limits instead of relying on implementations within this codebase? I will file an issue to track overriding the jackson maximums in case these configured amounts are over the max.

@reta
Copy link
Collaborator

reta commented Nov 20, 2023

Should I see if we can leverage jackson to enforce these limits instead of relying on implementations within this codebase?

We probably should just make sure those do not conflict

I will file an issue to track overriding the jackson maximums in case these configured amounts are over the max.

👍

@cwperks
Copy link
Member Author

cwperks commented Nov 20, 2023

@reta Capturing the discussion in an issue here. The jackson maximums look sensible to me and are much larger than then OpenSearch limits. Should OpenSearch allow configuring those values to values that exceed the new jackson maximums?

@reta
Copy link
Collaborator

reta commented Nov 20, 2023

Should OpenSearch allow configuring those values to values that exceed the new jackson maximums?

I will add context / details to the issue, thank you!

@reta reta merged commit 2b359ab into opensearch-project:main Nov 20, 2023
29 checks passed
@cwperks
Copy link
Member Author

cwperks commented Nov 20, 2023

@reta Can you add backport labels for 2.x and 1.x/1.3?

@reta reta added dependencies Pull requests that update a dependency file v3.0.0 Issues and PRs related to version 3.0.0 backport 2.x Backport to 2.x branch labels Nov 20, 2023
@reta
Copy link
Collaborator

reta commented Nov 20, 2023

@cwperks for 1.x it could be quite risky, it is still using 2.14.x

opensearch-trigger-bot bot pushed a commit that referenced this pull request Nov 20, 2023
* Bump jackson and jackson_databind from 2.15.2 to 2.16.0

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Add CHANGELOG entry

Signed-off-by: Craig Perkins <cwperx@amazon.com>

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
(cherry picked from commit 2b359ab)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
reta pushed a commit that referenced this pull request Nov 21, 2023
)

* Bump jackson and jackson_databind from 2.15.2 to 2.16.0



* Add CHANGELOG entry



---------


(cherry picked from commit 2b359ab)

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
fahadshamiinsta pushed a commit to fahadshamiinsta/OpenSearch270 that referenced this pull request Dec 4, 2023
…roject#11273)

* Bump jackson and jackson_databind from 2.15.2 to 2.16.0

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Add CHANGELOG entry

Signed-off-by: Craig Perkins <cwperx@amazon.com>

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
rayshrey pushed a commit to rayshrey/OpenSearch that referenced this pull request Mar 18, 2024
…roject#11273)

* Bump jackson and jackson_databind from 2.15.2 to 2.16.0

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Add CHANGELOG entry

Signed-off-by: Craig Perkins <cwperx@amazon.com>

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…roject#11273)

* Bump jackson and jackson_databind from 2.15.2 to 2.16.0

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Add CHANGELOG entry

Signed-off-by: Craig Perkins <cwperx@amazon.com>

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Shivansh Arora <hishiv@amazon.com>
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 dependencies Pull requests that update a dependency file v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants