Skip to content

Commit e3c9fc1

Browse files
committed
Remove the failures field from snapshot responses
Failure handling for snapshots was made stricter in #107191 (8.15), so this field is always empty since then. Clients don't need to check it anymore for failure handling, we can remove it from API responses in 9.0
1 parent 4eab631 commit e3c9fc1

File tree

4 files changed

+6
-65
lines changed

4 files changed

+6
-65
lines changed

server/src/internalClusterTest/java/org/elasticsearch/snapshots/SnapshotStatusApisIT.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,6 @@ public void testGetSnapshotsNoRepos() {
316316
.get();
317317

318318
assertTrue(getSnapshotsResponse.getSnapshots().isEmpty());
319-
assertTrue(getSnapshotsResponse.getFailures().isEmpty());
320319
}
321320

322321
public void testGetSnapshotsMultipleRepos() throws Exception {

server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java

Lines changed: 6 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,10 @@
1717
import org.elasticsearch.common.io.stream.StreamOutput;
1818
import org.elasticsearch.common.xcontent.ChunkedToXContentObject;
1919
import org.elasticsearch.core.Nullable;
20-
import org.elasticsearch.core.UpdateForV9;
2120
import org.elasticsearch.snapshots.SnapshotInfo;
2221
import org.elasticsearch.xcontent.ToXContent;
2322

2423
import java.io.IOException;
25-
import java.util.Collections;
2624
import java.util.Iterator;
2725
import java.util.List;
2826
import java.util.Map;
@@ -35,9 +33,6 @@ public class GetSnapshotsResponse extends ActionResponse implements ChunkedToXCo
3533

3634
private final List<SnapshotInfo> snapshots;
3735

38-
@UpdateForV9(owner = UpdateForV9.Owner.DISTRIBUTED_COORDINATION) // always empty, can be dropped
39-
private final Map<String, ElasticsearchException> failures;
40-
4136
@Nullable
4237
private final String next;
4338

@@ -53,15 +48,15 @@ public GetSnapshotsResponse(
5348
final int remaining
5449
) {
5550
this.snapshots = List.copyOf(snapshots);
56-
this.failures = failures == null ? Map.of() : Map.copyOf(failures);
5751
this.next = next;
5852
this.total = total;
5953
this.remaining = remaining;
6054
}
6155

6256
public GetSnapshotsResponse(StreamInput in) throws IOException {
6357
this.snapshots = in.readCollectionAsImmutableList(SnapshotInfo::readFrom);
64-
this.failures = Collections.unmodifiableMap(in.readMap(StreamInput::readException));
58+
// TODO Skip when we have V9 in TransportVersions
59+
in.readMap(StreamInput::readException);
6560
this.next = in.readOptionalString();
6661
this.total = in.readVInt();
6762
this.remaining = in.readVInt();
@@ -76,25 +71,11 @@ public List<SnapshotInfo> getSnapshots() {
7671
return snapshots;
7772
}
7873

79-
/**
80-
* Returns a map of repository name to {@link ElasticsearchException} for each unsuccessful response.
81-
*/
82-
public Map<String, ElasticsearchException> getFailures() {
83-
return failures;
84-
}
85-
8674
@Nullable
8775
public String next() {
8876
return next;
8977
}
9078

91-
/**
92-
* Returns true if there is at least one failed response.
93-
*/
94-
public boolean isFailed() {
95-
return failures.isEmpty() == false;
96-
}
97-
9879
public int totalCount() {
9980
return total;
10081
}
@@ -106,7 +87,8 @@ public int remaining() {
10687
@Override
10788
public void writeTo(StreamOutput out) throws IOException {
10889
out.writeCollection(snapshots);
109-
out.writeMap(failures, StreamOutput::writeException);
90+
// TODO Skip when we have V9 in TransportVersions
91+
out.writeMap(Map.of(), StreamOutput::writeException);
11092
out.writeOptionalString(next);
11193
out.writeVInt(total);
11294
out.writeVInt(remaining);
@@ -120,18 +102,6 @@ public Iterator<ToXContent> toXContentChunked(ToXContent.Params params) {
120102
return b;
121103
}), Iterators.map(getSnapshots().iterator(), snapshotInfo -> snapshotInfo::toXContentExternal), Iterators.single((b, p) -> {
122104
b.endArray();
123-
if (failures.isEmpty() == false) {
124-
b.startObject("failures");
125-
for (Map.Entry<String, ElasticsearchException> error : failures.entrySet()) {
126-
b.field(error.getKey(), (bb, pa) -> {
127-
bb.startObject();
128-
error.getValue().toXContent(bb, pa);
129-
bb.endObject();
130-
return bb;
131-
});
132-
}
133-
b.endObject();
134-
}
135105
if (next != null) {
136106
b.field("next", next);
137107
}
@@ -151,12 +121,12 @@ public boolean equals(Object o) {
151121
if (this == o) return true;
152122
if (o == null || getClass() != o.getClass()) return false;
153123
GetSnapshotsResponse that = (GetSnapshotsResponse) o;
154-
return Objects.equals(snapshots, that.snapshots) && Objects.equals(failures, that.failures) && Objects.equals(next, that.next);
124+
return Objects.equals(snapshots, that.snapshots) && Objects.equals(next, that.next);
155125
}
156126

157127
@Override
158128
public int hashCode() {
159-
return Objects.hash(snapshots, failures, next);
129+
return Objects.hash(snapshots, next);
160130
}
161131

162132
@Override

server/src/main/java/org/elasticsearch/rest/action/cat/RestSnapshotAction.java

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,9 @@
99

1010
package org.elasticsearch.rest.action.cat;
1111

12-
import org.elasticsearch.ElasticsearchException;
1312
import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsRequest;
1413
import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse;
1514
import org.elasticsearch.client.internal.node.NodeClient;
16-
import org.elasticsearch.common.Strings;
1715
import org.elasticsearch.common.Table;
1816
import org.elasticsearch.common.time.DateFormatter;
1917
import org.elasticsearch.core.TimeValue;
@@ -99,24 +97,6 @@ protected Table getTableWithHeader(RestRequest request) {
9997
private Table buildTable(RestRequest req, GetSnapshotsResponse getSnapshotsResponse) {
10098
Table table = getTableWithHeader(req);
10199

102-
if (getSnapshotsResponse.isFailed()) {
103-
ElasticsearchException causes = null;
104-
105-
for (ElasticsearchException e : getSnapshotsResponse.getFailures().values()) {
106-
if (causes == null) {
107-
causes = e;
108-
} else {
109-
causes.addSuppressed(e);
110-
}
111-
}
112-
throw new ElasticsearchException(
113-
"Repositories ["
114-
+ Strings.collectionToCommaDelimitedString(getSnapshotsResponse.getFailures().keySet())
115-
+ "] failed to retrieve snapshots",
116-
causes
117-
);
118-
}
119-
120100
for (SnapshotInfo snapshotStatus : getSnapshotsResponse.getSnapshots()) {
121101
table.startRow();
122102

server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,6 @@
3737
import java.util.Map;
3838
import java.util.Set;
3939

40-
import static org.hamcrest.CoreMatchers.containsString;
41-
4240
public class GetSnapshotsResponseTests extends ESTestCase {
4341
// We can not subclass AbstractSerializingTestCase because it
4442
// can only be used for instances with equals and hashCode
@@ -60,12 +58,6 @@ private GetSnapshotsResponse copyInstance(GetSnapshotsResponse instance) throws
6058
private void assertEqualInstances(GetSnapshotsResponse expectedInstance, GetSnapshotsResponse newInstance) {
6159
assertEquals(expectedInstance.getSnapshots(), newInstance.getSnapshots());
6260
assertEquals(expectedInstance.next(), newInstance.next());
63-
assertEquals(expectedInstance.getFailures().keySet(), newInstance.getFailures().keySet());
64-
for (Map.Entry<String, ElasticsearchException> expectedEntry : expectedInstance.getFailures().entrySet()) {
65-
ElasticsearchException expectedException = expectedEntry.getValue();
66-
ElasticsearchException newException = newInstance.getFailures().get(expectedEntry.getKey());
67-
assertThat(newException.getMessage(), containsString(expectedException.getMessage()));
68-
}
6961
}
7062

7163
private List<SnapshotInfo> createSnapshotInfos(String repoName) {

0 commit comments

Comments
 (0)