C0urante commented on PR #12295: URL: https://github.com/apache/kafka/pull/12295#issuecomment-1161074753
### Hanging tests Phew, you got me worried there for a sec! I wrote the zombie fencing unit tests and we just merged them a week or two ago. I did some digging and it looks like the tests are hanging during `PowerMock.mockStatic(RestClient.class);`, which is due to a known issue with PowerMock that has an easy workaround: https://github.com/powermock/powermock/issues/610 I was able to fix the tests locally by changing the class-level annotation from `@PrepareForTest({DistributedHerder.class, Plugins.class, RestClient.class})` to `@PrepareForTest({DistributedHerder.class, Plugins.class})`, and adding the annotation `@PrepareForTest({DistributedHerder.class, Plugins.class, RestClient.class})` to the `testTaskRequestedZombieFencingForwardedToLeader` and `testTaskRequestedZombieFencingFailedForwardToLeader` test methods. Can you give that a shot? I'm still not sure exactly why your new test affected these test cases. But, as long as your test works and the existing tests work, I'm fine with it. ### Invoking `onFailure` Regarding this question: > In `getConnectorStartingCallable`, immediate failures are passed to `onFailure`; is that something we’d have to account for? Great point--I do think that calling `onFailure` when exceptions are thrown is warranted; in this case, it'd cause us to mark the connector as `FAILED` instead of leaving it in whatever state it was in previously, which seems accurate. I wonder if we might actually leverage `getConnectorStartingCallable` in this method? At this point we're essentially duplicating its logic, and it's possible (though admittedly unlikely) that there would be multiple reconfigured connectors, in which case we'd be able to parallelize things and move faster. I'm imagining something like this, LMKWYT: ```java private void processConnectorConfigUpdates(Set<String> connectorConfigUpdates) { // ... Collection<Callable<Void>> connectorsToStart = new ArrayList<>(); // New for (String connectorName : connectorConfigUpdates) { // ... // The update may be a deletion, so verify we actually need to restart the connector if (remains) { connectorsToStart.add(getConnectorStartingCallable(connectorName)); // Replaces existing if-statement body } } startAndStop(connectorsToStart); // New } ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
