Skip to content

Commit fed07a5

Browse files
authored
Test bug in TimeSeriesIndexSearcherTests (#83730)
This test checks that documents are presented in tsid and then timestamp order to collectors; it was not taking a deep copy of the tsid and so when a change in tsid stayed on the same segment we could get inaccurate comparisons as the BytesRef would change out from underneath us and would compare as equal to the old timestamp, meaning that the check for strictly increasing timestamp would fail. Fixes #83647
1 parent 9d867cd commit fed07a5

File tree

2 files changed

+11
-6
lines changed

2 files changed

+11
-6
lines changed

server/src/main/java/org/elasticsearch/search/aggregations/timeseries/TimeSeriesIndexSearcher.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.apache.lucene.search.Weight;
2121
import org.apache.lucene.util.Bits;
2222
import org.apache.lucene.util.BytesRef;
23+
import org.apache.lucene.util.BytesRefBuilder;
2324
import org.apache.lucene.util.PriorityQueue;
2425
import org.elasticsearch.cluster.metadata.DataStream;
2526
import org.elasticsearch.index.mapper.TimeSeriesIdFieldMapper;
@@ -99,7 +100,7 @@ private boolean populateQueue(List<LeafWalker> leafWalkers, PriorityQueue<LeafWa
99100
it.remove();
100101
continue;
101102
}
102-
BytesRef tsid = leafWalker.tsids.lookupOrd(leafWalker.tsids.ordValue());
103+
BytesRef tsid = leafWalker.getTsid();
103104
if (currentTsid == null) {
104105
currentTsid = tsid;
105106
}
@@ -136,6 +137,7 @@ private static class LeafWalker {
136137
private final DocIdSetIterator iterator;
137138
private final SortedDocValues tsids;
138139
private final SortedNumericDocValues timestamps; // TODO can we have this just a NumericDocValues?
140+
private final BytesRefBuilder scratch = new BytesRefBuilder();
139141
int docId = -1;
140142
int tsidOrd;
141143
long timestamp;
@@ -168,6 +170,11 @@ int nextDoc() throws IOException {
168170
return docId;
169171
}
170172

173+
BytesRef getTsid() throws IOException {
174+
scratch.copyBytes(tsids.lookupOrd(tsids.ordValue()));
175+
return scratch.get();
176+
}
177+
171178
// invalid if the doc is deleted or if it doesn't have a tsid or timestamp entry
172179
private boolean isInvalidDoc(int docId) throws IOException {
173180
return (liveDocs != null && liveDocs.get(docId) == false)

server/src/test/java/org/elasticsearch/search/aggregations/timeseries/TimeSeriesIndexSearcherTests.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.apache.lucene.search.SortField;
2727
import org.apache.lucene.store.Directory;
2828
import org.apache.lucene.util.BytesRef;
29-
import org.apache.lucene.util.LuceneTestCase.AwaitsFix;
3029
import org.elasticsearch.cluster.metadata.DataStream;
3130
import org.elasticsearch.index.mapper.TimeSeriesIdFieldMapper;
3231
import org.elasticsearch.search.aggregations.BucketCollector;
@@ -40,7 +39,6 @@
4039
import java.util.concurrent.TimeUnit;
4140
import java.util.concurrent.atomic.AtomicInteger;
4241

43-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/83647")
4442
public class TimeSeriesIndexSearcherTests extends ESTestCase {
4543

4644
// Index a random set of docs with timestamp and tsid with the tsid/timestamp sort order
@@ -108,13 +106,13 @@ public void collect(int doc, long owningBucketOrd) throws IOException {
108106
BytesRef latestTSID = tsid.lookupOrd(tsid.ordValue());
109107
long latestTimestamp = timestamp.longValue();
110108
if (currentTSID != null) {
111-
assertTrue(latestTSID.compareTo(currentTSID) >= 0);
109+
assertTrue(currentTSID + "->" + latestTSID.utf8ToString(), latestTSID.compareTo(currentTSID) >= 0);
112110
if (latestTSID.equals(currentTSID)) {
113-
assertTrue(latestTimestamp >= currentTimestamp);
111+
assertTrue(currentTimestamp + "->" + latestTimestamp, latestTimestamp >= currentTimestamp);
114112
}
115113
}
116114
currentTimestamp = latestTimestamp;
117-
currentTSID = latestTSID;
115+
currentTSID = BytesRef.deepCopyOf(latestTSID);
118116
total++;
119117
}
120118
};

0 commit comments

Comments
 (0)