Skip to content

Commit 6fbc98b

Browse files
muffixmuffix
committed
Scale timestamps to milliseconds for split queries
Scales all timestamps for split queries to milliseconds. It's important to maintain consistent units between all the partial queries that make up the bigger one.
1 parent 18fab40 commit 6fbc98b

File tree

6 files changed

+100
-19
lines changed

6 files changed

+100
-19
lines changed

‎src/core/SplitRollupQuery.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public long getStartTime() {
6060
}
6161

6262
/**
63-
* Sets the start time of the graph.
63+
* Sets the start time of the graph. Converts the timestamp to milliseconds if necessary.
6464
*
6565
* @param timestamp The start time, all the data points returned will have a
6666
* timestamp greater than or equal to this one.
@@ -71,6 +71,8 @@ public long getStartTime() {
7171
*/
7272
@Override
7373
public void setStartTime(long timestamp) {
74+
if ((timestamp & Const.SECOND_MASK) == 0) { timestamp *= 1000L; }
75+
7476
if (rollupQuery == null) {
7577
rawQuery.setStartTime(timestamp);
7678
return;
@@ -101,7 +103,7 @@ public long getEndTime() {
101103
}
102104

103105
/**
104-
* Sets the end time of the graph.
106+
* Sets the end time of the graph. Converts the timestamp to milliseconds if necessary.
105107
*
106108
* @param timestamp The end time, all the data points returned will have a
107109
* timestamp less than or equal to this one.
@@ -112,6 +114,8 @@ public long getEndTime() {
112114
*/
113115
@Override
114116
public void setEndTime(long timestamp) {
117+
if ((timestamp & Const.SECOND_MASK) == 0) { timestamp *= 1000L; }
118+
115119
rawQuery.setEndTime(timestamp);
116120
}
117121

‎src/core/TsdbQuery.java‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -448,17 +448,44 @@ public Deferred<Object> split(final TSQuery query, final int index, final TsdbQu
448448

449449
Deferred<Object> rawResolutionDeferred = rawQuery.configureFromQuery(query, index, true);
450450

451-
long lastAvailableMillis = rollup_query.getLastRollupTimestampSeconds() * 1000L;
452-
boolean isRawOnlyQuery = lastAvailableMillis <= getStartTime();
451+
long lastRollupTimestampMillis = rollup_query.getLastRollupTimestampSeconds() * 1000L;
453452

453+
boolean isRawOnlyQuery = QueryUtil.isTimestampBefore(lastRollupTimestampMillis, getStartTime());
454454
if (!isRawOnlyQuery) {
455-
setEndTime(lastAvailableMillis);
456-
rawQuery.setStartTime(lastAvailableMillis);
455+
updateRollupSplitTimes(rawQuery, lastRollupTimestampMillis);
457456
}
458457

459458
return rawResolutionDeferred;
460459
}
461460

461+
/**
462+
* Updates the timestamp of this query and the corresponding raw part in the case of a split.
463+
*
464+
* Sets the start and end times for this query so that it hits the rollup table until the given timestamp.
465+
* Also updates the passed {@param rawQuery} with the new start time so that it hits the raw table for points from the
466+
* given timestamp onwards.
467+
*
468+
* Makes sure that all timestamps are in milliseconds.
469+
*
470+
* @param rawQuery The raw query part
471+
* @param splitTimestamp The timestamp until when rollup data is guaranteed to be available
472+
*/
473+
private void updateRollupSplitTimes(final TsdbQuery rawQuery, long splitTimestamp) {
474+
setEndTime(splitTimestamp);
475+
476+
boolean isStartTimeInSeconds = (getStartTime() & Const.SECOND_MASK) == 0;
477+
if (isStartTimeInSeconds) {
478+
setStartTime(getStartTime() * 1000L);
479+
}
480+
481+
boolean isRawEndTimeInSeconds = (rawQuery.getEndTime() & Const.SECOND_MASK) == 0;
482+
if (isRawEndTimeInSeconds) {
483+
rawQuery.setEndTime(rawQuery.getEndTime() * 1000L);
484+
}
485+
486+
rawQuery.setStartTime(splitTimestamp);
487+
}
488+
462489
@Override
463490
public Deferred<Object> configureFromQuery(final TSQuery query,
464491
final int index) {

‎src/query/QueryUtil.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,4 +403,24 @@ public static String byteRegexToString(final String regexp) {
403403
}
404404
return buf.toString();
405405
}
406+
407+
/**
408+
* Compares two timestamps where either can be in seconds or milliseconds.
409+
*
410+
* @param ts1 The first timestamp in either seconds or milliseconds.
411+
* @param ts2 The second timestamp in either seconds or milliseconds.
412+
* @return Whether the first timestamp is before the second
413+
*/
414+
public static boolean isTimestampBefore(long ts1, long ts2) {
415+
boolean ts1InSeconds = (ts1 & Const.SECOND_MASK) == 0;
416+
boolean ts2InSeconds = (ts2 & Const.SECOND_MASK) == 0;
417+
418+
if (ts1InSeconds && !ts2InSeconds) {
419+
ts1 *= 1000L;
420+
} else if (!ts1InSeconds && ts2InSeconds) {
421+
ts2 *= 1000L;
422+
}
423+
424+
return ts1 < ts2;
425+
}
406426
}

‎test/core/TestSplitRollupQuery.java‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ public void beforeLocal() {
4444
@Test
4545
public void setStartTime() {
4646
queryUnderTest.setStartTime(42L);
47-
assertEquals(42L, queryUnderTest.getStartTime());
48-
assertEquals(42L, rollupQuery.getStartTime());
47+
assertEquals(42000L, queryUnderTest.getStartTime());
48+
assertEquals(42000L, rollupQuery.getStartTime());
4949
}
5050

5151
@Test
@@ -56,7 +56,7 @@ public void setStartTimeBeyondOriginalEnd() {
5656
rollupQuery.setEndTime(41L);
5757
queryUnderTest.setStartTime(42L);
5858

59-
assertEquals(42L, queryUnderTest.getStartTime());
59+
assertEquals(42000L, queryUnderTest.getStartTime());
6060
}
6161

6262
@Test
@@ -67,28 +67,29 @@ public void setStartTimeWithoutRollupQuery() {
6767

6868
queryUnderTest.setStartTime(42L);
6969

70-
assertEquals(42L, queryUnderTest.getStartTime());
70+
assertEquals(42000L, queryUnderTest.getStartTime());
7171
}
7272

7373
@Test
7474
public void setEndTime() {
7575
TsdbQuery rawQuery = new TsdbQuery(tsdb);
7676
Whitebox.setInternalState(queryUnderTest, "rawQuery", rawQuery);
7777

78-
rollupQuery.setEndTime(21L);
79-
rawQuery.setStartTime(21L);
78+
rollupQuery.setStartTime(0);
79+
rollupQuery.setEndTime(21000L);
80+
rawQuery.setStartTime(21000L);
8081

8182
queryUnderTest.setEndTime(42L);
8283

83-
assertEquals(42L, queryUnderTest.getEndTime());
84-
assertEquals(21L, rollupQuery.getEndTime());
85-
assertEquals(42L, rawQuery.getEndTime());
84+
assertEquals(42000L, queryUnderTest.getEndTime());
85+
assertEquals(21000L, rollupQuery.getEndTime());
86+
assertEquals(42000L, rawQuery.getEndTime());
8687
}
8788

8889
@Test(expected = IllegalArgumentException.class)
8990
public void setEndTimeBeforeRawStartTime() {
9091
TsdbQuery rawQuery = new TsdbQuery(tsdb);
91-
rawQuery.setStartTime(84L);
92+
rawQuery.setStartTime(DateTime.currentTimeMillis());
9293

9394
Whitebox.setInternalState(queryUnderTest, "rawQuery", rawQuery);
9495

‎test/core/TestTsdbQuery.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@
5353
@RunWith(PowerMockRunner.class)
5454
@PrepareForTest({ DateTime.class, TsdbQuery.class })
5555
public final class TestTsdbQuery extends BaseTsdbTest {
56+
57+
private static final long ONE_DAY_MS = 24 * 60 * 60 * 1000;
58+
5659
private TsdbQuery query = null;
5760

5861
@Before
@@ -694,19 +697,25 @@ public void needsSplittingReturnsTrueIfStartAndEndInBlackoutPeriod() {
694697

695698
@Test
696699
public void split() {
697-
mockSystemTime(1356998400000L);
700+
long mockSystemTime = 1356998400000L;
701+
mockSystemTime(mockSystemTime);
698702
mockEnableRollupQuerySplitting();
699703

700704
TSQuery tsQuery = getTSQuery();
701705
TsdbQuery rawQuery = spy(new TsdbQuery(tsdb));
702706

703-
query.setStartTime(0);
704-
rawQuery.setStartTime(42);
707+
query.setStartTime(mockSystemTime - 7 * ONE_DAY_MS);
708+
705709
doReturn(Deferred.fromResult(null)).when(rawQuery).configureFromQuery(eq(tsQuery), eq(0), eq(true));
706710

707711
query.split(tsQuery, 0, rawQuery);
708712

709713
verify(rawQuery).configureFromQuery(eq(tsQuery), eq(0), eq(true));
714+
715+
assertEquals(mockSystemTime - 7 * ONE_DAY_MS, query.getStartTime());
716+
assertEquals(mockSystemTime - 2 * ONE_DAY_MS, query.getEndTime());
717+
assertEquals(mockSystemTime - 2 * ONE_DAY_MS, rawQuery.getStartTime());
718+
assertEquals(mockSystemTime, rawQuery.getEndTime());
710719
}
711720

712721
@Test(expected = IllegalStateException.class)

‎test/query/TestQueryUtil.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,17 @@
1212
// see <http://www.gnu.org/licenses/>.
1313
package net.opentsdb.query;
1414

15+
import static org.junit.Assert.assertFalse;
16+
import static org.junit.Assert.assertTrue;
1517
import static org.mockito.Matchers.any;
1618
import static org.mockito.Mockito.mock;
1719
import static org.mockito.Mockito.never;
1820
import static org.mockito.Mockito.times;
1921
import static org.mockito.Mockito.verify;
2022
import static org.mockito.Mockito.when;
2123

24+
import net.opentsdb.core.Query;
25+
import net.opentsdb.utils.DateTime;
2226
import org.hbase.async.Bytes.ByteMap;
2327
import org.hbase.async.FilterList;
2428
import org.hbase.async.KeyRegexpFilter;
@@ -145,4 +149,20 @@ public void setDataTableScanFilterEnableBoth() throws Exception {
145149
verify(scanner, times(1)).setStartKey(any(byte[].class));
146150
verify(scanner, times(1)).setStopKey(any(byte[].class));
147151
}
152+
153+
@Test
154+
public void timestampComparison() {
155+
long now = DateTime.currentTimeMillis() / 1000L;
156+
assertTrue(QueryUtil.isTimestampBefore(now*1000, now+1));
157+
assertTrue(QueryUtil.isTimestampBefore(now-1, now*1000L));
158+
assertTrue(QueryUtil.isTimestampBefore(now-1, now));
159+
assertTrue(QueryUtil.isTimestampBefore((now-1)*1000L, now*1000L));
160+
161+
assertFalse(QueryUtil.isTimestampBefore(now+1, now*1000L));
162+
assertFalse(QueryUtil.isTimestampBefore(now*1000L, now-1));
163+
assertFalse(QueryUtil.isTimestampBefore(now, now-1));
164+
assertFalse(QueryUtil.isTimestampBefore(now*1000L, (now-1)*1000L));
165+
166+
assertFalse(QueryUtil.isTimestampBefore(now, now));
167+
}
148168
}

0 commit comments

Comments
 (0)