Refactor AllocationAwarenessDecider

[patelsmit32123]

Description

Refactored AllocationAwarenessDecider class to have more accurate variable/method names

Related Issues

Check List

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]

❌ Gradle check result for f6ffcef: 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?

[patelsmit32123]

@Bukhtawar can you please help take a look? I was going through this piece of code and some of the variable names seemed a bit confusing. If the changes make sense, then i can polish the PR and add all necessary details.

I think with this PR, I can also improve the related documentation.

[Bukhtawar]

Thanks for the changes, I'll let @RS146BIJAY give it as pass as well.

[github-actions]

❌ Gradle check result for 57314d4: 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?

[github-actions]

❌ Gradle check result for cb1e302: 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?

[github-actions]

❌ Gradle check result for c6fb23c: 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?

[patelsmit32123]

@RS146BIJAY please review, answered all comments

[RS146BIJAY]

@patelsmit32123 Build seems to be breaking. Though I think both add within a loop and addAll should perform same, can we cross validate once how addAll performs vs a single add call on a Set within a loop?

[github-actions]

❌ Gradle check result for d2d0bf3: 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?

[github-actions]

✅ Gradle check result for 30c193b: SUCCESS

[codecov]

Codecov Report

Attention: Patch coverage is 95.23810% with 1 line in your changes missing coverage. Please review.

Project coverage is 71.95%. Comparing base (5642ce7) to head (30c193b).
Report is 180 commits behind head on main.

Files with missing lines	Patch %	Lines
...allocation/decider/AwarenessAllocationDecider.java	95.23%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #15856      +/-   ##
============================================
+ Coverage     71.92%   71.95%   +0.02%     
+ Complexity    64261    64223      -38     
============================================
  Files          5272     5272              
  Lines        300538   300536       -2     
  Branches      43432    43431       -1     
============================================
+ Hits         216169   216257      +88     
+ Misses        66631    66486     -145     
- Partials      17738    17793      +55     

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

[patelsmit32123]

@patelsmit32123 Build seems to be breaking. Though I think both add within a loop and addAll should perform same, can we cross validate once how addAll performs vs a single add call on a Set within a loop?

@RS146BIJAY I ran a simple benchmarking code with 1M entries, the addAll() method seems to be performing better
https://ideone.com/Qv9vaf

I have also fixed the build

[patelsmit32123]

@sachinpkale can you please review? or @RS146BIJAY please tag a relevant maintainer to review, thanks

[RS146BIJAY]

Changes looks ok to me. Adding more folks who can review it.
@ashking94 @gbbafna

[patelsmit32123]

@ashking94 @gbbafna please take a look when you can

[ashking94]

@patelsmit32123 The refactoring done makes some of the variable names and method names really long. Also, the meaning does not change much when you are going through the code before and after your change. I would like to know the purpose of this change?

Should we consider adding java doc if it is to understand what the method does?

[opensearch-trigger-bot]

This PR is stalled because it has been open for 30 days with no activity.