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

[Backport 2.11] Performance Improvement for Datetime formats to 2.11 #10453

Conversation

CaptainDredge
Copy link
Contributor

@CaptainDredge CaptainDredge commented Oct 6, 2023

Description

Backport changes #9567 and #10385

Related Issues

Resolves #4558

Public doc: opensearch-project/documentation-website#5163

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)
  • 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.

…project#9567)

* Added performance improvement for datetime field parsing

This adds caching of formatters in case of no explicit format specified for the datetime field in mapping.
This also adds `strict_date_time_no_millis` as additional formatter in default date time formats

Signed-off-by: Prabhat Sharma <[email protected]>

* Refactor DateTimeFormatter Access under featireflag

Signed-off-by: Prabhat Sharma <[email protected]>

---------

Signed-off-by: Prabhat Sharma <[email protected]>
Co-authored-by: Prabhat Sharma <[email protected]>
(cherry picked from commit 2965e69)
Signed-off-by: Prabhat Sharma <[email protected]>
(cherry picked from commit 5a459ba)
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

Compatibility status:

Checks if related components are compatible with change fbddd58

Incompatible components

Skipped components

Compatible components

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

* Race condition fix for datetime optimization

Signed-off-by: Prabhat Sharma <[email protected]>

* Changed JavaDateTimeFormatter caching of parser from MRU(most recently used) to a simple last used formatter

Signed-off-by: Prabhat Sharma <[email protected]>

---------

Signed-off-by: Prabhat Sharma <[email protected]>
Co-authored-by: Prabhat Sharma <[email protected]>
Signed-off-by: Prabhat Sharma <[email protected]>
(cherry picked from commit ad17b38)
Signed-off-by: Prabhat Sharma <[email protected]>
@CaptainDredge CaptainDredge force-pushed the backport/backport-9567-to-2.11 branch from 4112eba to fbddd58 Compare October 6, 2023 11:09
@CaptainDredge
Copy link
Contributor Author

Trigerring build for flaky test to pass

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

Gradle Check (Jenkins) Run Completed with:

@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

Merging #10453 (fbddd58) into 2.11 (f164bf5) will increase coverage by 0.06%.
Report is 2 commits behind head on 2.11.
The diff coverage is 79.64%.

@@             Coverage Diff              @@
##               2.11   #10453      +/-   ##
============================================
+ Coverage     70.86%   70.92%   +0.06%     
- Complexity    58501    58573      +72     
============================================
  Files          4829     4829              
  Lines        276344   276424      +80     
  Branches      40578    40589      +11     
============================================
+ Hits         195821   196046     +225     
+ Misses        63808    63720      -88     
+ Partials      16715    16658      -57     
Files Coverage Δ
...nsearch/http/netty4/Netty4HttpServerTransport.java 75.75% <100.00%> (+0.56%) ⬆️
...in/java/org/opensearch/transport/Netty4Plugin.java 73.68% <ø> (ø)
...pensearch/common/settings/FeatureFlagSettings.java 50.00% <ø> (ø)
.../org/opensearch/index/mapper/RangeFieldMapper.java 87.25% <100.00%> (ø)
.../org/opensearch/index/mapper/RootObjectMapper.java 82.74% <100.00%> (+0.07%) ⬆️
...in/java/org/opensearch/index/shard/IndexShard.java 68.95% <100.00%> (-0.40%) ⬇️
...ices/replication/RemoteStoreReplicationSource.java 90.62% <100.00%> (+0.14%) ⬆️
...src/main/java/org/opensearch/rest/RestHandler.java 60.37% <ø> (ø)
...n/java/org/opensearch/script/ScoreScriptUtils.java 1.50% <100.00%> (ø)
...nsearch/search/aggregations/support/ValueType.java 79.59% <100.00%> (+0.42%) ⬆️
... and 9 more

... and 446 files with indirect coverage changes

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

I’m pretty sure 2.11 only takes bug fixes since yesterday, but @CEHENKLE can correct me if I’m wrong

@reta
Copy link
Collaborator

reta commented Oct 6, 2023

I’m pretty sure 2.11 only takes bug fixes since yesterday, but @CEHENKLE can correct me if I’m wrong

That's is correct, this is a backport for 2.x (2.12)

@CEHENKLE
Copy link
Member

CEHENKLE commented Oct 6, 2023

Yeah, the only place it gets weird is when we haven't cut the release branch yet, and by default 2.x is the release branch. So unless it creates problems/more work for the release managers (@bbarani or @gaiksaya or @rishabh6788 or @peterzhuamazon or anyone else :) ) then it's fine with me :)

@dblock
Copy link
Member

dblock commented Oct 6, 2023

That's is correct, this is a backport for 2.x (2.12)

This one is into a branch called 2.11, so should we not take it here and close this PR?

@msfroh
Copy link
Collaborator

msfroh commented Oct 9, 2023

This one is into a branch called 2.11, so should we not take it here and close this PR?

Yeah -- I'm going to go ahead and close this one, since it sounds like we're all on the same page about not backporting to 2.11. (2.x is fine.)

Feel free to reopen if I'm wrong.

@msfroh msfroh closed this Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants