Skip to content

Commit

Permalink
Remove timeStampField parameter from DataStream constructors. (#85309)
Browse files Browse the repository at this point in the history
Timestamp field is always '@timestamp' and is already fixed to this
field. Removing this parameter from DataStream's constructors makes this
clearer and makes the code a bit cleaner too.

This change doesn't break bwc in any way or change the xcontent/binary
format. A next followup change may do this.

Followup from #85260
  • Loading branch information
martijnvg authored Mar 28, 2022
1 parent ea8f180 commit e8498dc
Show file tree
Hide file tree
Showing 13 changed files with 32 additions and 143 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ public void testGetAdditionalIndexSettingsDataStreamAlreadyCreatedTimeSettingsMi
mb.put(
new DataStream(
ds.getName(),
ds.getTimeStampField(),
ds.getIndices(),
ds.getGeneration(),
ds.getMetadata(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ public void testRolloverClusterStateForDataStream() throws Exception {
String dataStreamName = "logs-my-app";
final DataStream dataStream = new DataStream(
dataStreamName,
new DataStream.TimestampField("@timestamp"),
List.of(new Index(DataStream.getDefaultBackingIndexName(dataStreamName, 1, now.toEpochMilli()), "uuid")),
1,
null,
Expand Down Expand Up @@ -152,7 +151,6 @@ public void testRolloverAndMigrateDataStream() throws Exception {
IndexMode dsIndexMode = randomBoolean() ? null : IndexMode.STANDARD;
final DataStream dataStream = new DataStream(
dataStreamName,
new DataStream.TimestampField("@timestamp"),
List.of(new Index(DataStream.getDefaultBackingIndexName(dataStreamName, 1, now.toEpochMilli()), "uuid")),
1,
null,
Expand Down Expand Up @@ -237,7 +235,6 @@ public void testChangingIndexModeFromTimeSeriesToSomethingElseNoEffectOnExisting
String dataStreamName = "logs-my-app";
final DataStream dataStream = new DataStream(
dataStreamName,
new DataStream.TimestampField("@timestamp"),
List.of(new Index(DataStream.getDefaultBackingIndexName(dataStreamName, 1, now.toEpochMilli()), "uuid")),
1,
null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ public void testUpdateTimeSeriesTemporalRange_NoUpdateBecauseReplicated() {
.put(
new DataStream(
d.getName(),
d.getTimeStampField(),
d.getIndices(),
d.getGeneration(),
d.getMetadata(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public final class DataStream implements SimpleDiffable<DataStream>, ToXContentO

public static final String BACKING_INDEX_PREFIX = ".ds-";
public static final DateFormatter DATE_FORMATTER = DateFormatter.forPattern("uuuu.MM.dd");
public static final TimestampField TIMESTAMP_FIELD = new DataStream.TimestampField("@timestamp");
// Timeseries indices' leaf readers should be sorted by desc order of their timestamp field, as it allows search time optimizations
public static Comparator<LeafReader> TIMESERIES_LEAF_READERS_SORTER = Comparator.comparingLong((LeafReader r) -> {
try {
Expand All @@ -73,7 +74,6 @@ public final class DataStream implements SimpleDiffable<DataStream>, ToXContentO

private final LongSupplier timeProvider;
private final String name;
private final TimestampField timeStampField;
private final List<Index> indices;
private final long generation;
private final Map<String, Object> metadata;
Expand All @@ -85,7 +85,6 @@ public final class DataStream implements SimpleDiffable<DataStream>, ToXContentO

public DataStream(
String name,
TimestampField timeStampField,
List<Index> indices,
long generation,
Map<String, Object> metadata,
Expand All @@ -95,25 +94,12 @@ public DataStream(
boolean allowCustomRouting,
IndexMode indexMode
) {
this(
name,
timeStampField,
indices,
generation,
metadata,
hidden,
replicated,
system,
System::currentTimeMillis,
allowCustomRouting,
indexMode
);
this(name, indices, generation, metadata, hidden, replicated, system, System::currentTimeMillis, allowCustomRouting, indexMode);
}

// visible for testing
DataStream(
String name,
TimestampField timeStampField,
List<Index> indices,
long generation,
Map<String, Object> metadata,
Expand All @@ -125,7 +111,6 @@ public DataStream(
IndexMode indexMode
) {
this.name = name;
this.timeStampField = timeStampField;
this.indices = Collections.unmodifiableList(indices);
this.generation = generation;
this.metadata = metadata;
Expand All @@ -144,7 +129,8 @@ public String getName() {
}

public TimestampField getTimeStampField() {
return timeStampField;
// This was always fixed to @timestamp with the idea that one day this field could be configurable. This idea no longer exists.
return TIMESTAMP_FIELD;
}

public List<Index> getIndices() {
Expand Down Expand Up @@ -302,18 +288,7 @@ public DataStream unsafeRollover(Index writeIndex, long generation, IndexMode in

List<Index> backingIndices = new ArrayList<>(indices);
backingIndices.add(writeIndex);
return new DataStream(
name,
timeStampField,
backingIndices,
generation,
metadata,
hidden,
false,
system,
allowCustomRouting,
indexMode
);
return new DataStream(name, backingIndices, generation, metadata, hidden, false, system, allowCustomRouting, indexMode);
}

/**
Expand Down Expand Up @@ -378,18 +353,7 @@ public DataStream removeBackingIndex(Index index) {
List<Index> backingIndices = new ArrayList<>(indices);
backingIndices.remove(index);
assert backingIndices.size() == indices.size() - 1;
return new DataStream(
name,
timeStampField,
backingIndices,
generation + 1,
metadata,
hidden,
replicated,
system,
allowCustomRouting,
indexMode
);
return new DataStream(name, backingIndices, generation + 1, metadata, hidden, replicated, system, allowCustomRouting, indexMode);
}

/**
Expand Down Expand Up @@ -421,18 +385,7 @@ public DataStream replaceBackingIndex(Index existingBackingIndex, Index newBacki
);
}
backingIndices.set(backingIndexPosition, newBackingIndex);
return new DataStream(
name,
timeStampField,
backingIndices,
generation + 1,
metadata,
hidden,
replicated,
system,
allowCustomRouting,
indexMode
);
return new DataStream(name, backingIndices, generation + 1, metadata, hidden, replicated, system, allowCustomRouting, indexMode);
}

/**
Expand Down Expand Up @@ -479,34 +432,11 @@ public DataStream addBackingIndex(Metadata clusterMetadata, Index index) {
List<Index> backingIndices = new ArrayList<>(indices);
backingIndices.add(0, index);
assert backingIndices.size() == indices.size() + 1;
return new DataStream(
name,
timeStampField,
backingIndices,
generation + 1,
metadata,
hidden,
replicated,
system,
allowCustomRouting,
indexMode
);
return new DataStream(name, backingIndices, generation + 1, metadata, hidden, replicated, system, allowCustomRouting, indexMode);
}

public DataStream promoteDataStream() {
return new DataStream(
name,
timeStampField,
indices,
getGeneration(),
metadata,
hidden,
false,
system,
timeProvider,
allowCustomRouting,
indexMode
);
return new DataStream(name, indices, getGeneration(), metadata, hidden, false, system, timeProvider, allowCustomRouting, indexMode);
}

/**
Expand All @@ -531,7 +461,6 @@ public DataStream snapshot(Collection<String> indicesInSnapshot) {

return new DataStream(
name,
timeStampField,
reconciledIndices,
generation,
metadata == null ? null : new HashMap<>(metadata),
Expand Down Expand Up @@ -577,8 +506,7 @@ public static String getDefaultBackingIndexName(String dataStreamName, long gene
public DataStream(StreamInput in) throws IOException {
this(
in.readString(),
new TimestampField(in),
in.readList(Index::new),
readIndices(in),
in.readVLong(),
in.readMap(),
in.readBoolean(),
Expand All @@ -589,14 +517,19 @@ public DataStream(StreamInput in) throws IOException {
);
}

static List<Index> readIndices(StreamInput in) throws IOException {
in.readString(); // timestamp field, which is always @timestamp
return in.readList(Index::new);
}

public static Diff<DataStream> readDiffFrom(StreamInput in) throws IOException {
return SimpleDiffable.readDiffFrom(DataStream::new, in);
}

@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeString(name);
timeStampField.writeTo(out);
TIMESTAMP_FIELD.writeTo(out);
out.writeList(indices);
out.writeVLong(generation);
out.writeGenericMap(metadata);
Expand All @@ -623,11 +556,10 @@ public void writeTo(StreamOutput out) throws IOException {
public static final ParseField INDEX_MODE = new ParseField("index_mode");

@SuppressWarnings("unchecked")
private static final ConstructingObjectParser<DataStream, Void> PARSER = new ConstructingObjectParser<>(
"data_stream",
args -> new DataStream(
private static final ConstructingObjectParser<DataStream, Void> PARSER = new ConstructingObjectParser<>("data_stream", args -> {
assert TIMESTAMP_FIELD == args[1];
return new DataStream(
(String) args[0],
(TimestampField) args[1],
(List<Index>) args[2],
(Long) args[3],
(Map<String, Object>) args[4],
Expand All @@ -636,8 +568,8 @@ public void writeTo(StreamOutput out) throws IOException {
args[7] != null && (boolean) args[7],
args[8] != null && (boolean) args[8],
args[9] != null ? IndexMode.fromString((String) args[9]) : null
)
);
);
});

static {
PARSER.declareString(ConstructingObjectParser.constructorArg(), NAME_FIELD);
Expand All @@ -660,7 +592,7 @@ public static DataStream fromXContent(XContentParser parser) throws IOException
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.field(NAME_FIELD.getPreferredName(), name);
builder.field(TIMESTAMP_FIELD_FIELD.getPreferredName(), timeStampField);
builder.field(TIMESTAMP_FIELD_FIELD.getPreferredName(), TIMESTAMP_FIELD);
builder.xContentList(INDICES_FIELD.getPreferredName(), indices);
builder.field(GENERATION_FIELD.getPreferredName(), generation);
if (metadata != null) {
Expand All @@ -683,7 +615,6 @@ public boolean equals(Object o) {
if (o == null || getClass() != o.getClass()) return false;
DataStream that = (DataStream) o;
return name.equals(that.name)
&& timeStampField.equals(that.timeStampField)
&& indices.equals(that.indices)
&& generation == that.generation
&& Objects.equals(metadata, that.metadata)
Expand All @@ -695,7 +626,7 @@ public boolean equals(Object o) {

@Override
public int hashCode() {
return Objects.hash(name, timeStampField, indices, generation, metadata, hidden, replicated, allowCustomRouting, indexMode);
return Objects.hash(name, indices, generation, metadata, hidden, replicated, allowCustomRouting, indexMode);
}

public static final class TimestampField implements Writeable, ToXContentObject {
Expand All @@ -707,7 +638,12 @@ public static final class TimestampField implements Writeable, ToXContentObject
@SuppressWarnings("unchecked")
private static final ConstructingObjectParser<TimestampField, Void> PARSER = new ConstructingObjectParser<>(
"timestamp_field",
args -> new TimestampField((String) args[0])
args -> {
if (FIXED_TIMESTAMP_FIELD.equals(args[0]) == false) {
throw new IllegalArgumentException("unexpected timestamp field [" + args[0] + "]");
}
return TIMESTAMP_FIELD;
}
);

static {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,6 @@ static ClusterState createDataStream(
final IndexMode indexMode = template.getDataStreamTemplate().getIndexMode();
DataStream newDataStream = new DataStream(
dataStreamName,
timestampField,
dsBackingIndices,
1L,
template.metadata() != null ? Map.copyOf(template.metadata()) : null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,6 @@ static DataStream updateDataStream(DataStream dataStream, Metadata.Builder metad
.toList();
return new DataStream(
dataStreamName,
dataStream.getTimeStampField(),
updatedIndices,
dataStream.getGeneration(),
dataStream.getMetadata(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ public void testRolloverIndexMode() {
// Unsure index_mode=null
ds = new DataStream(
ds.getName(),
ds.getTimeStampField(),
ds.getIndices(),
ds.getGeneration(),
ds.getMetadata(),
Expand All @@ -127,7 +126,6 @@ public void testRolloverIndexMode_keepIndexMode() {
DataStream ds = DataStreamTestHelper.randomInstance().promoteDataStream();
ds = new DataStream(
ds.getName(),
ds.getTimeStampField(),
ds.getIndices(),
ds.getGeneration(),
ds.getMetadata(),
Expand Down Expand Up @@ -489,7 +487,6 @@ public void testSnapshot() {

var postSnapshotDataStream = new DataStream(
preSnapshotDataStream.getName(),
preSnapshotDataStream.getTimeStampField(),
postSnapshotIndices,
preSnapshotDataStream.getGeneration() + randomIntBetween(0, 5),
preSnapshotDataStream.getMetadata() == null ? null : new HashMap<>(preSnapshotDataStream.getMetadata()),
Expand Down Expand Up @@ -532,7 +529,6 @@ public void testSnapshotWithAllBackingIndicesRemoved() {

var postSnapshotDataStream = new DataStream(
preSnapshotDataStream.getName(),
preSnapshotDataStream.getTimeStampField(),
indicesToAdd,
preSnapshotDataStream.getGeneration(),
preSnapshotDataStream.getMetadata(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@

import static org.elasticsearch.cluster.metadata.DataStreamTestHelper.backingIndexEqualTo;
import static org.elasticsearch.cluster.metadata.DataStreamTestHelper.createBackingIndex;
import static org.elasticsearch.cluster.metadata.DataStreamTestHelper.createTimestampField;
import static org.elasticsearch.cluster.metadata.DataStreamTestHelper.newInstance;
import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_HIDDEN_SETTING;
import static org.elasticsearch.common.util.set.Sets.newHashSet;
Expand Down Expand Up @@ -2777,7 +2776,6 @@ public void testHiddenDataStreams() {
.put(
new DataStream(
dataStream1,
createTimestampField("@timestamp"),
List.of(index1.getIndex(), index2.getIndex()),
2,
Collections.emptyMap(),
Expand Down
Loading

0 comments on commit e8498dc

Please sign in to comment.