Skip to content

Commit 89d7c57

Browse files
Fix Incorrect Transport Response Handler Type (#38264)
* Fix Incorrect Transport Response Handler Type * The response type here is not empty and was always wrong but this only became visible now that 0a604e3 was introduced * As a result of 0a604e3 we started actually handling the response of this request and logging/handling exceptions before that we simply dropped the classcast exception here quietly using the empty response handler * fix busy assert not handling `Exception` * Closes #38226 * Closes #38256
1 parent 0861dc3 commit 89d7c57

File tree

3 files changed

+23
-8
lines changed

3 files changed

+23
-8
lines changed

server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,9 @@
6767
import org.elasticsearch.repositories.IndexId;
6868
import org.elasticsearch.repositories.Repository;
6969
import org.elasticsearch.threadpool.ThreadPool;
70-
import org.elasticsearch.transport.EmptyTransportResponseHandler;
7170
import org.elasticsearch.transport.TransportException;
7271
import org.elasticsearch.transport.TransportRequestDeduplicator;
73-
import org.elasticsearch.transport.TransportResponse;
72+
import org.elasticsearch.transport.TransportResponseHandler;
7473
import org.elasticsearch.transport.TransportService;
7574

7675
import java.io.IOException;
@@ -508,16 +507,28 @@ public void onFailure(Exception e) {
508507
}
509508
},
510509
(req, reqListener) -> transportService.sendRequest(transportService.getLocalNode(), UPDATE_SNAPSHOT_STATUS_ACTION_NAME, req,
511-
new EmptyTransportResponseHandler(ThreadPool.Names.SAME) {
510+
new TransportResponseHandler<UpdateIndexShardSnapshotStatusResponse>() {
512511
@Override
513-
public void handleResponse(TransportResponse.Empty response) {
512+
public UpdateIndexShardSnapshotStatusResponse read(StreamInput in) throws IOException {
513+
final UpdateIndexShardSnapshotStatusResponse response = new UpdateIndexShardSnapshotStatusResponse();
514+
response.readFrom(in);
515+
return response;
516+
}
517+
518+
@Override
519+
public void handleResponse(UpdateIndexShardSnapshotStatusResponse response) {
514520
reqListener.onResponse(null);
515521
}
516522

517523
@Override
518524
public void handleException(TransportException exp) {
519525
reqListener.onFailure(exp);
520526
}
527+
528+
@Override
529+
public String executor() {
530+
return ThreadPool.Names.SAME;
531+
}
521532
})
522533
);
523534
}

server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -988,7 +988,6 @@ public void testMasterShutdownDuringFailedSnapshot() throws Exception {
988988
* can be restored when the node the shrunken index was created on is no longer part of
989989
* the cluster.
990990
*/
991-
@AwaitsFix(bugUrl="https://github.com/elastic/elasticsearch/issues/38256")
992991
public void testRestoreShrinkIndex() throws Exception {
993992
logger.info("--> starting a master node and a data node");
994993
internalCluster().startMasterOnlyNode();

server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3637,7 +3637,6 @@ public void testParallelRestoreOperationsFromSingleSnapshot() throws Exception {
36373637
}
36383638

36393639
@TestLogging("org.elasticsearch.snapshots:TRACE")
3640-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/38226")
36413640
public void testAbortedSnapshotDuringInitDoesNotStart() throws Exception {
36423641
final Client client = client();
36433642

@@ -3684,8 +3683,14 @@ public void testAbortedSnapshotDuringInitDoesNotStart() throws Exception {
36843683

36853684
// The deletion must set the snapshot in the ABORTED state
36863685
assertBusy(() -> {
3687-
SnapshotsStatusResponse status = client.admin().cluster().prepareSnapshotStatus("repository").setSnapshots("snap").get();
3688-
assertThat(status.getSnapshots().iterator().next().getState(), equalTo(State.ABORTED));
3686+
try {
3687+
SnapshotsStatusResponse status =
3688+
client.admin().cluster().prepareSnapshotStatus("repository").setSnapshots("snap").get();
3689+
assertThat(status.getSnapshots().iterator().next().getState(), equalTo(State.ABORTED));
3690+
} catch (Exception e) {
3691+
// Force assertBusy to retry on every exception
3692+
throw new AssertionError(e);
3693+
}
36893694
});
36903695

36913696
// Now unblock the repository

0 commit comments

Comments
 (0)