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

Remove the failures field from snapshot responses #114496

Merged
merged 16 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,6 @@ public void testGetSnapshotsNoRepos() {
.get();

assertTrue(getSnapshotsResponse.getSnapshots().isEmpty());
assertTrue(getSnapshotsResponse.getFailures().isEmpty());
}

public void testGetSnapshotsMultipleRepos() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ChunkedToXContentObject;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.UpdateForV9;
import org.elasticsearch.snapshots.SnapshotInfo;
import org.elasticsearch.xcontent.ToXContent;

import java.io.IOException;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
Expand All @@ -35,9 +33,6 @@ public class GetSnapshotsResponse extends ActionResponse implements ChunkedToXCo

private final List<SnapshotInfo> snapshots;

@UpdateForV9(owner = UpdateForV9.Owner.DISTRIBUTED_COORDINATION) // always empty, can be dropped
private final Map<String, ElasticsearchException> failures;

@Nullable
private final String next;

Expand All @@ -53,15 +48,15 @@ public GetSnapshotsResponse(
final int remaining
) {
this.snapshots = List.copyOf(snapshots);
this.failures = failures == null ? Map.of() : Map.copyOf(failures);
this.next = next;
this.total = total;
this.remaining = remaining;
}

public GetSnapshotsResponse(StreamInput in) throws IOException {
this.snapshots = in.readCollectionAsImmutableList(SnapshotInfo::readFrom);
this.failures = Collections.unmodifiableMap(in.readMap(StreamInput::readException));
// TODO Skip when we have V9 in TransportVersions
in.readMap(StreamInput::readException);
this.next = in.readOptionalString();
this.total = in.readVInt();
this.remaining = in.readVInt();
Expand All @@ -76,25 +71,11 @@ public List<SnapshotInfo> getSnapshots() {
return snapshots;
}

/**
* Returns a map of repository name to {@link ElasticsearchException} for each unsuccessful response.
*/
public Map<String, ElasticsearchException> getFailures() {
return failures;
}

@Nullable
public String next() {
return next;
}

/**
* Returns true if there is at least one failed response.
*/
public boolean isFailed() {
return failures.isEmpty() == false;
}

public int totalCount() {
return total;
}
Expand All @@ -106,7 +87,8 @@ public int remaining() {
@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeCollection(snapshots);
out.writeMap(failures, StreamOutput::writeException);
// TODO Skip when we have V9 in TransportVersions
out.writeMap(Map.of(), StreamOutput::writeException);
out.writeOptionalString(next);
out.writeVInt(total);
out.writeVInt(remaining);
Expand All @@ -120,18 +102,6 @@ public Iterator<ToXContent> toXContentChunked(ToXContent.Params params) {
return b;
}), Iterators.map(getSnapshots().iterator(), snapshotInfo -> snapshotInfo::toXContentExternal), Iterators.single((b, p) -> {
b.endArray();
if (failures.isEmpty() == false) {
b.startObject("failures");
for (Map.Entry<String, ElasticsearchException> error : failures.entrySet()) {
b.field(error.getKey(), (bb, pa) -> {
bb.startObject();
error.getValue().toXContent(bb, pa);
bb.endObject();
return bb;
});
}
b.endObject();
}
if (next != null) {
b.field("next", next);
}
Expand All @@ -151,12 +121,12 @@ public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
GetSnapshotsResponse that = (GetSnapshotsResponse) o;
return Objects.equals(snapshots, that.snapshots) && Objects.equals(failures, that.failures) && Objects.equals(next, that.next);
return Objects.equals(snapshots, that.snapshots) && Objects.equals(next, that.next);
}

@Override
public int hashCode() {
return Objects.hash(snapshots, failures, next);
return Objects.hash(snapshots, next);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@

package org.elasticsearch.rest.action.cat;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsRequest;
import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse;
import org.elasticsearch.client.internal.node.NodeClient;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.Table;
import org.elasticsearch.common.time.DateFormatter;
import org.elasticsearch.core.TimeValue;
Expand Down Expand Up @@ -99,24 +97,6 @@ protected Table getTableWithHeader(RestRequest request) {
private Table buildTable(RestRequest req, GetSnapshotsResponse getSnapshotsResponse) {
Table table = getTableWithHeader(req);

if (getSnapshotsResponse.isFailed()) {
ElasticsearchException causes = null;

for (ElasticsearchException e : getSnapshotsResponse.getFailures().values()) {
if (causes == null) {
causes = e;
} else {
causes.addSuppressed(e);
}
}
throw new ElasticsearchException(
"Repositories ["
+ Strings.collectionToCommaDelimitedString(getSnapshotsResponse.getFailures().keySet())
+ "] failed to retrieve snapshots",
causes
);
}

for (SnapshotInfo snapshotStatus : getSnapshotsResponse.getSnapshots()) {
table.startRow();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@
import java.util.Map;
import java.util.Set;

import static org.hamcrest.CoreMatchers.containsString;

public class GetSnapshotsResponseTests extends ESTestCase {
// We can not subclass AbstractSerializingTestCase because it
// can only be used for instances with equals and hashCode
Expand All @@ -60,12 +58,6 @@ private GetSnapshotsResponse copyInstance(GetSnapshotsResponse instance) throws
private void assertEqualInstances(GetSnapshotsResponse expectedInstance, GetSnapshotsResponse newInstance) {
assertEquals(expectedInstance.getSnapshots(), newInstance.getSnapshots());
assertEquals(expectedInstance.next(), newInstance.next());
assertEquals(expectedInstance.getFailures().keySet(), newInstance.getFailures().keySet());
for (Map.Entry<String, ElasticsearchException> expectedEntry : expectedInstance.getFailures().entrySet()) {
ElasticsearchException expectedException = expectedEntry.getValue();
ElasticsearchException newException = newInstance.getFailures().get(expectedEntry.getKey());
assertThat(newException.getMessage(), containsString(expectedException.getMessage()));
}
}

private List<SnapshotInfo> createSnapshotInfos(String repoName) {
Expand Down