Skip to content

Conversation

masseyke
Copy link
Member

@masseyke masseyke commented Apr 6, 2023

BulkProcessor2RetryIT.testBulkRejectionLoadWithBackoff was failing for a couple of reasons:
(1) The test cluster sometimes intentionally times out requests, which can lead to failures.
(2) The client() method sometimes calls a method that triggers an assertion failure if you're running in a transport thread (which part of our test is):

        java.lang.AssertionError: Expected current thread [Thread[elasticsearch[node_s2][transport_worker][T#1],5,TGRP-BulkProcessor2RetryIT]] to not be a transport thread. Reason: [Blocking operation]
            at __randomizedtesting.SeedInfo.seed([7979906EFF34169B]:0)
            at org.elasticsearch.transport.Transports.assertNotTransportThread(Transports.java:56)
            at org.elasticsearch.common.util.concurrent.BaseFuture.blockingAllowed(BaseFuture.java:80)
            at org.elasticsearch.common.util.concurrent.BaseFuture.get(BaseFuture.java:74)
            at org.elasticsearch.common.util.concurrent.FutureUtils.get(FutureUtils.java:45)
            at org.elasticsearch.action.support.AdapterActionFuture.actionGet(AdapterActionFuture.java:26)
            at org.elasticsearch.action.support.PlainActionFuture.get(PlainActionFuture.java:24)
            at org.elasticsearch.transport.TransportService.openConnection(TransportService.java:483)
            at org.elasticsearch.client.transport.TransportClientNodesService$SimpleNodeSampler.doSample(TransportClientNodesService.java:413)
            at org.elasticsearch.client.transport.TransportClientNodesService$NodeSampler.sample(TransportClientNodesService.java:364)
            at org.elasticsearch.client.transport.TransportClientNodesService.addTransportAddresses(TransportClientNodesService.java:200)
            at org.elasticsearch.client.transport.TransportClient.addTransportAddress(TransportClient.java:416)
            at org.elasticsearch.test.InternalTestCluster$TransportClientFactory.client(InternalTestCluster.java:1224)
            at org.elasticsearch.test.InternalTestCluster$NodeAndClient.getOrBuildTransportClient(InternalTestCluster.java:1040)
            at org.elasticsearch.test.InternalTestCluster$NodeAndClient.client(InternalTestCluster.java:994)
            at org.elasticsearch.test.InternalTestCluster.client(InternalTestCluster.java:849)
            at org.elasticsearch.test.ESIntegTestCase.client(ESIntegTestCase.java:677)
            at org.elasticsearch.test.ESIntegTestCase.client(ESIntegTestCase.java:670)
            at org.elasticsearch.action.bulk.BulkProcessor2RetryIT.countAndBulk(BulkProcessor2RetryIT.java:167)

This PR fixes both of those.
Closes #94452
Closes #94941

@masseyke masseyke added >test Issues or PRs that are addressing/adding tests :Data Management/ILM+SLM Index and Snapshot lifecycle management v7.17.10 labels Apr 6, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@elasticsearchmachine elasticsearchmachine added the Team:Data Management Meta label for data/management team label Apr 6, 2023
Copy link
Contributor

@gmarouli gmarouli left a comment

Choose a reason for hiding this comment

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

LGTM, but please take into consideration that my knowledge is a bit shallow. If you just needed another pair of eyes, then all good :).

// @TestLogging(
// value = "org.elasticsearch.action.bulk.Retry2:trace",
// reason = "Logging information about locks useful for tracking down deadlock"
// )
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this TestLogging still useful? or should we remove it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I probably ought to remove it. I come here from time to time to grab this for use in other tests because I can never remember the syntax. :)

@masseyke masseyke merged commit 6a1c7e4 into elastic:7.17 Apr 11, 2023
@masseyke masseyke deleted the fixing-BulkProcessor2RetryIT branch April 11, 2023 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/ILM+SLM Index and Snapshot lifecycle management Team:Data Management Meta label for data/management team >test Issues or PRs that are addressing/adding tests v7.17.10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants