Skip to content

Revert "Index stats enhancement: creation date and tier_preference (#116339)" #116959

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.

Sign up for GitHub

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

Merged
DaveCTurner merged 2 commits into from
Nov 18, 2024
Merged

Revert "Index stats enhancement: creation date and tier_preference (#116339)" #116959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
037b18e
Revert "Index stats enhancement: creation date and tier_preference (#…
DaveCTurner Nov 18, 2024
00f4b56
Rename feature constant
DaveCTurner Nov 18, 2024
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
5 changes: 0 additions & 5 deletions docs/changelog/116339.yaml
View file
Open in desktop

This file was deleted.

14 changes: 0 additions & 14 deletions ...lRestTest/resources/rest-api-spec/test/indices.stats/16_creation_date_tier_preference.yml
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions server/src/main/java/org/elasticsearch/TransportVersions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ static TransportVersion def(int id) {
public static final TransportVersion SKIP_INNER_HITS_SEARCH_SOURCE = def(8_791_00_0);
public static final TransportVersion QUERY_RULES_LIST_INCLUDES_TYPES = def(8_792_00_0);
public static final TransportVersion INDEX_STATS_ADDITIONAL_FIELDS = def(8_793_00_0);
public static final TransportVersion INDEX_STATS_ADDITIONAL_FIELDS_REVERT = def(8_794_00_0);

/*
* STOP! READ THIS FIRST! No, really,
Expand Down
42 changes: 4 additions & 38 deletions server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndexStats.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@

public class IndexStats implements Iterable<IndexShardStats> {

public static final NodeFeature TIER_CREATION_DATE = new NodeFeature("stats.tier_creation_date");
// feature was effectively reverted but we still need to keep this constant around
public static final NodeFeature REVERTED_TIER_CREATION_DATE = new NodeFeature("stats.tier_creation_date");

private final String index;

Expand All @@ -32,27 +33,19 @@ public class IndexStats implements Iterable<IndexShardStats> {

private final IndexMetadata.State state;

private final List<String> tierPreference;

private final Long creationDate;

private final ShardStats shards[];

public IndexStats(
String index,
String uuid,
@Nullable ClusterHealthStatus health,
@Nullable IndexMetadata.State state,
@Nullable List<String> tierPreference,
@Nullable Long creationDate,
ShardStats[] shards
) {
this.index = index;
this.uuid = uuid;
this.health = health;
this.state = state;
this.tierPreference = tierPreference;
this.creationDate = creationDate;
this.shards = shards;
}

Expand All @@ -72,14 +65,6 @@ public IndexMetadata.State getState() {
return state;
}

public List<String> getTierPreference() {
return tierPreference;
}

public Long getCreationDate() {
return creationDate;
}

public ShardStats[] getShards() {
return this.shards;
}
Expand Down Expand Up @@ -148,24 +133,13 @@ public static class IndexStatsBuilder {
private final String uuid;
private final ClusterHealthStatus health;
private final IndexMetadata.State state;
private final List<String> tierPreference;
private final Long creationDate;
private final List<ShardStats> shards = new ArrayList<>();

public IndexStatsBuilder(
String indexName,
String uuid,
@Nullable ClusterHealthStatus health,
@Nullable IndexMetadata.State state,
@Nullable List<String> tierPreference,
@Nullable Long creationDate
) {
public IndexStatsBuilder(String indexName, String uuid, @Nullable ClusterHealthStatus health, @Nullable IndexMetadata.State state) {
this.indexName = indexName;
this.uuid = uuid;
this.health = health;
this.state = state;
this.tierPreference = tierPreference;
this.creationDate = creationDate;
}

public IndexStatsBuilder add(ShardStats shardStats) {
Expand All @@ -174,15 +148,7 @@ public IndexStatsBuilder add(ShardStats shardStats) {
}

public IndexStats build() {
return new IndexStats(
indexName,
uuid,
health,
state,
tierPreference,
creationDate,
shards.toArray(new ShardStats[shards.size()])
);
return new IndexStats(indexName, uuid, health, state, shards.toArray(new ShardStats[shards.size()]));
}
}
}
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsFeatures.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ public class IndicesStatsFeatures implements FeatureSpecification {

@Override
public Set<NodeFeature> getFeatures() {
return Set.of(IndexStats.TIER_CREATION_DATE);
return Set.of(IndexStats.REVERTED_TIER_CREATION_DATE);
}
}
48 changes: 13 additions & 35 deletions server/src/main/java/org/elasticsearch/action/admin/indices/stats/IndicesStatsResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,34 +47,29 @@ public class IndicesStatsResponse extends ChunkedBroadcastResponse {

private final Map<String, IndexMetadata.State> indexStateMap;

private final Map<String, List<String>> indexTierPreferenceMap;

private final Map<String, Long> indexCreationDateMap;

private final ShardStats[] shards;

private Map<ShardRouting, ShardStats> shardStatsMap;

IndicesStatsResponse(StreamInput in) throws IOException {
super(in);
shards = in.readArray(ShardStats::new, ShardStats[]::new);
if (in.getTransportVersion().onOrAfter(TransportVersions.INDEX_STATS_ADDITIONAL_FIELDS)) {
if (in.getTransportVersion().onOrAfter(TransportVersions.INDEX_STATS_ADDITIONAL_FIELDS_REVERT)) {
indexHealthMap = in.readMap(ClusterHealthStatus::readFrom);
indexStateMap = in.readMap(IndexMetadata.State::readFrom);
indexTierPreferenceMap = in.readMap(StreamInput::readStringCollectionAsList);
indexCreationDateMap = in.readMap(StreamInput::readLong);
} else if (in.getTransportVersion().onOrAfter(TransportVersions.INDEX_STATS_ADDITIONAL_FIELDS)) {
indexHealthMap = in.readMap(ClusterHealthStatus::readFrom);
indexStateMap = in.readMap(IndexMetadata.State::readFrom);
in.readMap(StreamInput::readStringCollectionAsList); // unused, reverted
in.readMap(StreamInput::readLong); // unused, reverted
} else if (in.getTransportVersion().onOrAfter(TransportVersions.V_8_1_0)) {
// Between 8.1 and INDEX_STATS_ADDITIONAL_FIELDS, we had a different format for the response
// where we only had health and state available.
indexHealthMap = in.readMap(ClusterHealthStatus::readFrom);
indexStateMap = in.readMap(IndexMetadata.State::readFrom);
indexTierPreferenceMap = Map.of();
indexCreationDateMap = Map.of();
} else {
indexHealthMap = Map.of();
indexStateMap = Map.of();
indexTierPreferenceMap = Map.of();
indexCreationDateMap = Map.of();
}
}

Expand All @@ -94,8 +89,6 @@ public class IndicesStatsResponse extends ChunkedBroadcastResponse {
Objects.requireNonNull(shards);
Map<String, ClusterHealthStatus> indexHealthModifiableMap = new HashMap<>();
Map<String, IndexMetadata.State> indexStateModifiableMap = new HashMap<>();
Map<String, List<String>> indexTierPreferenceModifiableMap = new HashMap<>();
Map<String, Long> indexCreationDateModifiableMap = new HashMap<>();
for (ShardStats shard : shards) {
Index index = shard.getShardRouting().index();
IndexMetadata indexMetadata = metadata.index(index);
Expand All @@ -105,14 +98,10 @@ public class IndicesStatsResponse extends ChunkedBroadcastResponse {
ignored -> new ClusterIndexHealth(indexMetadata, routingTable.index(index)).getStatus()
);
indexStateModifiableMap.computeIfAbsent(index.getName(), ignored -> indexMetadata.getState());
indexTierPreferenceModifiableMap.computeIfAbsent(index.getName(), ignored -> indexMetadata.getTierPreference());
indexCreationDateModifiableMap.computeIfAbsent(index.getName(), ignored -> indexMetadata.getCreationDate());
}
}
indexHealthMap = unmodifiableMap(indexHealthModifiableMap);
indexStateMap = unmodifiableMap(indexStateModifiableMap);
indexTierPreferenceMap = unmodifiableMap(indexTierPreferenceModifiableMap);
indexCreationDateMap = unmodifiableMap(indexCreationDateModifiableMap);
}

public Map<ShardRouting, ShardStats> asMap() {
Expand Down Expand Up @@ -150,14 +139,7 @@ public Map<String, IndexStats> getIndices() {
Index index = shard.getShardRouting().index();
IndexStatsBuilder indexStatsBuilder = indexToIndexStatsBuilder.computeIfAbsent(
index.getName(),
k -> new IndexStatsBuilder(
k,
index.getUUID(),
indexHealthMap.get(index.getName()),
indexStateMap.get(index.getName()),
indexTierPreferenceMap.get(index.getName()),
indexCreationDateMap.get(index.getName())
)
k -> new IndexStatsBuilder(k, index.getUUID(), indexHealthMap.get(index.getName()), indexStateMap.get(index.getName()))
);
indexStatsBuilder.add(shard);
}
Expand Down Expand Up @@ -202,12 +184,14 @@ public CommonStats getPrimaries() {
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeArray(shards);
if (out.getTransportVersion().onOrAfter(TransportVersions.INDEX_STATS_ADDITIONAL_FIELDS)) {
if (out.getTransportVersion().onOrAfter(TransportVersions.INDEX_STATS_ADDITIONAL_FIELDS_REVERT)) {
out.writeMap(indexHealthMap, StreamOutput::writeWriteable);
out.writeMap(indexStateMap, StreamOutput::writeWriteable);
out.writeMap(indexTierPreferenceMap, StreamOutput::writeStringCollection);
out.writeMap(indexCreationDateMap, StreamOutput::writeLong);

} else if (out.getTransportVersion().onOrAfter(TransportVersions.INDEX_STATS_ADDITIONAL_FIELDS)) {
out.writeMap(indexHealthMap, StreamOutput::writeWriteable);
out.writeMap(indexStateMap, StreamOutput::writeWriteable);
out.writeMap(Map.of(), StreamOutput::writeStringCollection);
out.writeMap(Map.of(), StreamOutput::writeLong);
Comment on lines +193 to +194
Copy link
Copy Markdown
Member

@dakrone dakrone Nov 18, 2024

Choose a reason for hiding this comment

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

Should we add some kind of check to be able to eventually remove these?

Copy link
Copy Markdown
Member Author

@DaveCTurner DaveCTurner Nov 18, 2024

Choose a reason for hiding this comment

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

Yeah eventually all these transport versions get collapsed into a release-version-named one (see e.g. org.elasticsearch.TransportVersions#V_8_15_2) and then this branch will be dead code:

        if (in.getTransportVersion().onOrAfter(TransportVersions.V_8_16_x)) {
            indexHealthMap = in.readMap(ClusterHealthStatus::readFrom);
            indexStateMap = in.readMap(IndexMetadata.State::readFrom);
        } else if (in.getTransportVersion().onOrAfter(TransportVersions.V_8_16_x)) {
            // same condition as the previous if so cannot get here

} else if (out.getTransportVersion().onOrAfter(TransportVersions.V_8_1_0)) {
out.writeMap(indexHealthMap, StreamOutput::writeWriteable);
out.writeMap(indexStateMap, StreamOutput::writeWriteable);
Expand Down Expand Up @@ -237,12 +221,6 @@ protected Iterator<ToXContent> customXContentChunks(ToXContent.Params params) {
if (indexStats.getState() != null) {
builder.field("status", indexStats.getState().toString().toLowerCase(Locale.ROOT));
}
if (indexStats.getTierPreference() != null) {
builder.field("tier_preference", indexStats.getTierPreference());
}
if (indexStats.getCreationDate() != null) {
builder.field("creation_date", indexStats.getCreationDate());
}
Comment on lines -240 to -245
Copy link
Copy Markdown
Member Author

@DaveCTurner DaveCTurner Nov 18, 2024

Choose a reason for hiding this comment

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

I believe this is ok, I don't think any clients could have been expecting these fields yet and they were optional anyway so we can remove them without ceremony.

builder.startObject("primaries");
indexStats.getPrimaries().toXContent(builder, p);
builder.endObject();
Expand Down
7 changes: 2 additions & 5 deletions ...ugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/WaitForNoFollowersStepTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,10 +189,7 @@ public void testNoShardStats() {
shardStats[0] = sStats;

mockXPackInfo(true, true);
mockIndexStatsCall(
indexName,
new IndexStats(indexName, "uuid", ClusterHealthStatus.GREEN, IndexMetadata.State.OPEN, null, null, shardStats)
);
mockIndexStatsCall(indexName, new IndexStats(indexName, "uuid", ClusterHealthStatus.GREEN, IndexMetadata.State.OPEN, shardStats));

final SetOnce<Boolean> conditionMetHolder = new SetOnce<>();
final SetOnce<ToXContentObject> stepInfoHolder = new SetOnce<>();
Expand Down Expand Up @@ -292,7 +289,7 @@ private IndexStats randomIndexStats(boolean isLeaderIndex, int numOfShards) {
for (int i = 0; i < numOfShards; i++) {
shardStats[i] = randomShardStats(isLeaderIndex);
}
return new IndexStats(randomAlphaOfLength(5), randomAlphaOfLength(10), null, null, null, null, shardStats);
return new IndexStats(randomAlphaOfLength(5), randomAlphaOfLength(10), null, null, shardStats);
}

private ShardStats randomShardStats(boolean isLeaderIndex) {
Expand Down
2 changes: 0 additions & 2 deletions .../org/elasticsearch/xpack/monitoring/collector/indices/IndicesStatsMonitoringDocTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ public void setUp() throws Exception {
"dcvO5uZATE-EhIKc3tk9Bg",
null,
null,
null,
null,
new ShardStats[] {
// Primaries
new ShardStats(mockShardRouting(true), mockShardPath(), mockCommonStats(), null, null, null, false, 0),
Expand Down