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 to 2.x] Bump BWC Version to 2.18 and Fix Bugs (#1311) #1312

Merged
merged 1 commit into from
Sep 14, 2024

Conversation

kaituo
Copy link
Collaborator

@kaituo kaituo commented Sep 13, 2024

backport #1311 to 2.x

This PR includes the following updates and bug fixes:

* Bump BWC Version to 2.18: Updated BWC version to 2.18 since the 2.17 branch has been cut.
* Fix Confidence Value Exceeding 1 in RCF: Addressed a bug in RCF where the confidence value could exceed 1. Implemented a check to cap the confidence value at 1, preventing invalid confidence scores.
* Correct Parameter Assignment in GetAnomalyDetectorTransportAction: Fixed an issue where parameter assignments within a method did not affect external variables due to Java's pass-by-value nature.
* Fixed a bug in ResultProcessor where we were supposed to check whether the number of sent messages equals the number of received messages before starting imputation. However, the sent message count was mistakenly based on the number of pages rather than the actual number of messages.
* Fixed a bug where we mistakenly used the total reserved memory bytes as the memory size per entity in PriorityCache.

Testing done:
* added test cases for the buggy scenarios
* manual e2e testing

Signed-off-by: Kaituo Li <[email protected]>
Copy link

codecov bot commented Sep 13, 2024

Codecov Report

Attention: Patch coverage is 52.94118% with 8 lines in your changes missing coverage. Please review.

Project coverage is 77.82%. Comparing base (138dbd0) to head (b3d033b).
Report is 23 commits behind head on 2.x.

Files with missing lines Patch % Lines
...ensearch/timeseries/transport/ResultProcessor.java 12.50% 7 Missing ⚠️
...d/transport/GetAnomalyDetectorTransportAction.java 50.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #1312      +/-   ##
============================================
+ Coverage     71.40%   77.82%   +6.42%     
- Complexity     4878     5476     +598     
============================================
  Files           518      533      +15     
  Lines         22931    23313     +382     
  Branches       2260     2309      +49     
============================================
+ Hits          16373    18143    +1770     
+ Misses         5509     4118    -1391     
- Partials       1049     1052       +3     
Flag Coverage Δ
plugin 77.82% <52.94%> (+6.42%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...in/java/org/opensearch/ad/model/AnomalyResult.java 80.28% <100.00%> (-5.08%) ⬇️
.../org/opensearch/forecast/model/ForecastResult.java 74.74% <100.00%> (+1.30%) ⬆️
...g/opensearch/timeseries/caching/PriorityCache.java 69.54% <100.00%> (+0.39%) ⬆️
...series/transport/BaseGetConfigTransportAction.java 86.26% <100.00%> (+10.43%) ⬆️
...d/transport/GetAnomalyDetectorTransportAction.java 86.66% <50.00%> (+6.66%) ⬆️
...ensearch/timeseries/transport/ResultProcessor.java 75.82% <12.50%> (-1.65%) ⬇️

... and 110 files with indirect coverage changes

@dbwiddis dbwiddis merged commit c518a7c into opensearch-project:2.x Sep 14, 2024
22 of 29 checks passed
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.

2 participants