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

[2.x] Fix TransportService constructor due to changes in core plus guava bump #1069

Merged
merged 4 commits into from
Oct 5, 2023

Conversation

amitgalitz
Copy link
Member

@amitgalitz amitgalitz commented Oct 4, 2023

Description

Add a tracer field for the TransportService constructor in our test cases

Core change to add tracer in MockNioTransport https://github.com/opensearch-project/OpenSearch/pull/10143/files#diff-f37f7447c76ba79901c3298a1addce39d103ce72efc5a75a1f7602a4e2829b62R115

core changes to add tracker in TransportService opensearch-project/OpenSearch@27cddec

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: Amit Galitzky <amgalitz@amazon.com>
@amitgalitz
Copy link
Member Author

amitgalitz commented Oct 5, 2023

Every test except org.opensearch.ad.task.ADTaskManagerTests.testScaleTaskLaneOnCoordinatingNode passes locally after a couple retries, not sure why we are seeing such an increase in flaky tests. More prevalent on GitHub even. This fix however does solve compilation issues

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>
ohltyler
ohltyler previously approved these changes Oct 5, 2023
@owaiskazi19
Copy link
Member

Shouldn't we get this change on main and then backport to 2.x?

@amitgalitz
Copy link
Member Author

https://github.com/opensearch-project/OpenSearch/pull/10143/files#diff-f37f7447c76ba79901c3298a1addce39d103ce72efc5a75a1f7602a4e2829b62R115

I usually agree its just that main is pretty broken right now with other things so I am trying to get it fixed on 2.x and 2.11 first then I can forward port this change

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>
ohltyler
ohltyler previously approved these changes Oct 5, 2023
@amitgalitz amitgalitz changed the title [2.x] Fix TransportService constructor due to changes in core [2.x] Fix TransportService constructor due to changes in core plus guava bump Oct 5, 2023
owaiskazi19
owaiskazi19 previously approved these changes Oct 5, 2023
Signed-off-by: Amit Galitzky <amgalitz@amazon.com>
@amitgalitz amitgalitz dismissed stale reviews from owaiskazi19 and ohltyler via 811ac33 October 5, 2023 19:22
@amitgalitz amitgalitz merged commit f81b75d into opensearch-project:2.x Oct 5, 2023
12 of 13 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 5, 2023
…ava bump (#1069)

* adjust transportService constructor due to changes in core

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>

* updated guava and fixed another compile issue

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>

* fixed mocking of transport service

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>

* adding 2.11 release notes

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>

---------

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>
(cherry picked from commit f81b75d)
ohltyler pushed a commit that referenced this pull request Oct 5, 2023
…ava bump (#1069) (#1070)

* adjust transportService constructor due to changes in core

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>

* updated guava and fixed another compile issue

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>

* fixed mocking of transport service

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>

* adding 2.11 release notes

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>

---------

Signed-off-by: Amit Galitzky <amgalitz@amazon.com>
(cherry picked from commit f81b75d)

Co-authored-by: Amit Galitzky <amgalitz@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants