Skip to content

Commit 8e9228f

Browse files
lohanidamodarlohanidamodar
committed
refactor(adapter): drop setUseDailyRollups flag; routing is automatic
The routing decision is a pure function of the parsed request shape. Requests that match a rollup MV's columns and don't ask for a sub-day interval route to the MV; everything else returns 'raw' and behaves exactly as before. Removing the flag eliminates a footgun (silent no-op via method_exists guard on the wrapper) and simplifies the cloud caller — bumping the library version alone delivers the win, no opt-in call needed. setDualReadSampleRate() remains as an explicit rollout-week debugging opt-in.
1 parent 720af37 commit 8e9228f

7 files changed

Lines changed: 24 additions & 187 deletions

File tree

‎src/Usage/Adapter/ClickHouse.php‎

Lines changed: 5 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -130,16 +130,8 @@ class ClickHouse extends SQL
130130
private ?string $nextQueryId = null;
131131

132132
/**
133-
* Route flat-sum event reads to the daily rollup table(s) when the
134-
* caller hasn't asked for any grouping. Default off — opt-in per
135-
* consumer via setUseDailyRollups(true).
136-
*/
137-
private bool $useDailyRollups = false;
138-
139-
/**
140-
* Structured log entries recorded for each routing decision when
141-
* useDailyRollups is active. Ops dashboards read these to confirm
142-
* rollup hit-rate.
133+
* Structured log entries recorded for each routing decision. Ops
134+
* dashboards read these to confirm rollup hit-rate.
143135
*
144136
* @var array<array{operation: string, metric: ?string, route_decision: string, start: ?string, end: ?string, dimensions: array<int, string>, interval: ?string}>
145137
*/
@@ -298,28 +290,6 @@ public function setNextQueryId(?string $queryId): self
298290
return $this;
299291
}
300292

301-
/**
302-
* Toggle daily-rollup routing for flat-sum event reads.
303-
*
304-
* When enabled, sum() against TYPE_EVENT may route to the daily MV (or
305-
* a hybrid daily + raw UNION ALL when the window straddles today)
306-
* whenever the request shape has no grouping / interval and only
307-
* touches columns the daily MV indexes. Everything else falls back to
308-
* the raw events table.
309-
*
310-
* Default off — cloud consumers opt in explicitly.
311-
*/
312-
public function setUseDailyRollups(bool $enabled = true): self
313-
{
314-
$this->useDailyRollups = $enabled;
315-
return $this;
316-
}
317-
318-
public function getUseDailyRollups(): bool
319-
{
320-
return $this->useDailyRollups;
321-
}
322-
323293
/**
324294
* Return the in-memory route-decision log (operation, metric,
325295
* route_decision, window, dimensions, interval). Cleared by
@@ -1894,7 +1864,7 @@ public function find(array $queries = [], ?string $type = null): array
18941864
$this->setOperationContext('find()');
18951865

18961866
if ($type !== null) {
1897-
if ($this->useDailyRollups && $type === Usage::TYPE_EVENT) {
1867+
if ($type === Usage::TYPE_EVENT) {
18981868
$plan = $this->extractRoutingPlan($queries);
18991869
$route = $this->selectAggregateSource($plan, Usage::TYPE_EVENT);
19001870
$this->recordRoute('find', $plan, $route);
@@ -1919,7 +1889,7 @@ public function find(array $queries = [], ?string $type = null): array
19191889
}
19201890
}
19211891

1922-
if ($this->useDailyRollups && $type === Usage::TYPE_GAUGE) {
1892+
if ($type === Usage::TYPE_GAUGE) {
19231893
$plan = $this->extractRoutingPlan($queries);
19241894
$route = $this->selectAggregateSource($plan, Usage::TYPE_GAUGE);
19251895
$this->recordRoute('find', $plan, $route);
@@ -2368,7 +2338,7 @@ public function sum(array $queries = [], string $attribute = 'value', string $ty
23682338
{
23692339
$this->setOperationContext('sum()');
23702340

2371-
if ($this->useDailyRollups && $type === Usage::TYPE_EVENT && $attribute === 'value') {
2341+
if ($type === Usage::TYPE_EVENT && $attribute === 'value') {
23722342
$plan = $this->extractRoutingPlan($queries);
23732343
$route = $this->selectAggregateSource($plan);
23742344
$this->recordRoute('sum', $plan, $route);

‎src/Usage/Usage.php‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -320,20 +320,6 @@ public function setSharedTables(bool $sharedTables): self
320320
return $this;
321321
}
322322

323-
/**
324-
* Opt into adapter-side daily-rollup routing for flat-sum event reads.
325-
*
326-
* Forwarded to the underlying adapter; only adapters that implement
327-
* `setUseDailyRollups()` honour the flag (ClickHouse does today).
328-
*/
329-
public function setUseDailyRollups(bool $enabled = true): self
330-
{
331-
if (method_exists($this->adapter, 'setUseDailyRollups')) {
332-
$this->adapter->setUseDailyRollups($enabled);
333-
}
334-
return $this;
335-
}
336-
337323
/**
338324
* Set the dual-read sampling rate for the adapter. Forwarded to the
339325
* underlying adapter; only adapters that implement

‎tests/Benchmark/EventsBench.php‎

Lines changed: 4 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -41,29 +41,6 @@ public function testBenchmarks(): void
4141
], Usage::TYPE_EVENT);
4242
});
4343

44-
$this->runBench('bench_events_topN_path_30d', function (string $queryId) use ($start, $end): void {
45-
$this->adapter->setNextQueryId($queryId);
46-
$this->usage->find([
47-
UsageQuery::groupBy('path'),
48-
Query::equal('metric', [$this->metric]),
49-
Query::greaterThanEqual('time', $start),
50-
Query::lessThanEqual('time', $end),
51-
Query::limit(500),
52-
], Usage::TYPE_EVENT);
53-
});
54-
55-
$this->runBench('bench_events_topN_method_status_30d', function (string $queryId) use ($start, $end): void {
56-
$this->adapter->setNextQueryId($queryId);
57-
$this->usage->find([
58-
UsageQuery::groupBy('method'),
59-
UsageQuery::groupBy('status'),
60-
Query::equal('metric', [$this->metric]),
61-
Query::greaterThanEqual('time', $start),
62-
Query::lessThanEqual('time', $end),
63-
Query::limit(200),
64-
], Usage::TYPE_EVENT);
65-
});
66-
6744
$this->runBench('bench_events_count_max_5k', function (string $queryId) use ($start, $end): void {
6845
$this->adapter->setNextQueryId($queryId);
6946
$this->usage->count([
@@ -93,37 +70,7 @@ public function testBenchmarks(): void
9370
$this->usage->addBatch($batch, Usage::TYPE_EVENT);
9471
}, 3);
9572

96-
// Multi-dim MV target scenarios — until P3.3 routes through the MVs
97-
// these still scan raw events, providing the baseline numbers that
98-
// commit 5 will compare against.
99-
$this->runBench('bench_events_topN_country_30d', function (string $queryId) use ($start, $end): void {
100-
$this->adapter->setNextQueryId($queryId);
101-
$this->usage->find([
102-
UsageQuery::groupBy('country'),
103-
Query::equal('metric', [$this->metric]),
104-
Query::greaterThanEqual('time', $start),
105-
Query::lessThanEqual('time', $end),
106-
Query::limit(200),
107-
], Usage::TYPE_EVENT);
108-
});
109-
110-
$this->runBench('bench_events_topN_service_30d', function (string $queryId) use ($start, $end): void {
111-
$this->adapter->setNextQueryId($queryId);
112-
$this->usage->find([
113-
UsageQuery::groupBy('service'),
114-
Query::equal('metric', [$this->metric]),
115-
Query::greaterThanEqual('time', $start),
116-
Query::lessThanEqual('time', $end),
117-
Query::limit(200),
118-
], Usage::TYPE_EVENT);
119-
});
120-
121-
// Multi-dim MV scenarios — bench_*_mv variants exercise the same
122-
// request shape with setUseDailyRollups on. Commit 5 wires the
123-
// routing through; until then both variants scan raw and produce
124-
// the same numbers.
125-
$this->runBench('bench_events_topN_path_30d_mv', function (string $queryId) use ($start, $end): void {
126-
$this->adapter->setUseDailyRollups(true);
73+
$this->runBench('bench_events_topN_path_30d', function (string $queryId) use ($start, $end): void {
12774
$this->adapter->setNextQueryId($queryId);
12875
$this->usage->find([
12976
UsageQuery::groupBy('path'),
@@ -132,11 +79,9 @@ public function testBenchmarks(): void
13279
Query::lessThanEqual('time', $end),
13380
Query::limit(500),
13481
], Usage::TYPE_EVENT);
135-
$this->adapter->setUseDailyRollups(false);
13682
});
13783

138-
$this->runBench('bench_events_topN_country_30d_mv', function (string $queryId) use ($start, $end): void {
139-
$this->adapter->setUseDailyRollups(true);
84+
$this->runBench('bench_events_topN_country_30d', function (string $queryId) use ($start, $end): void {
14085
$this->adapter->setNextQueryId($queryId);
14186
$this->usage->find([
14287
UsageQuery::groupBy('country'),
@@ -145,11 +90,9 @@ public function testBenchmarks(): void
14590
Query::lessThanEqual('time', $end),
14691
Query::limit(200),
14792
], Usage::TYPE_EVENT);
148-
$this->adapter->setUseDailyRollups(false);
14993
});
15094

151-
$this->runBench('bench_events_topN_service_30d_mv', function (string $queryId) use ($start, $end): void {
152-
$this->adapter->setUseDailyRollups(true);
95+
$this->runBench('bench_events_topN_service_30d', function (string $queryId) use ($start, $end): void {
15396
$this->adapter->setNextQueryId($queryId);
15497
$this->usage->find([
15598
UsageQuery::groupBy('service'),
@@ -158,11 +101,9 @@ public function testBenchmarks(): void
158101
Query::lessThanEqual('time', $end),
159102
Query::limit(200),
160103
], Usage::TYPE_EVENT);
161-
$this->adapter->setUseDailyRollups(false);
162104
});
163105

164-
$this->runBench('bench_events_topN_method_status_30d_mv', function (string $queryId) use ($start, $end): void {
165-
$this->adapter->setUseDailyRollups(true);
106+
$this->runBench('bench_events_topN_method_status_30d', function (string $queryId) use ($start, $end): void {
166107
$this->adapter->setNextQueryId($queryId);
167108
$this->usage->find([
168109
UsageQuery::groupBy('method'),
@@ -172,13 +113,11 @@ public function testBenchmarks(): void
172113
Query::lessThanEqual('time', $end),
173114
Query::limit(200),
174115
], Usage::TYPE_EVENT);
175-
$this->adapter->setUseDailyRollups(false);
176116
});
177117

178118
$todayStart = (new \DateTime('today', new \DateTimeZone('UTC')))->format('Y-m-d H:i:s');
179119
$todayEnd = (new \DateTime('+2 hour'))->format('Y-m-d H:i:s');
180120
$this->runBench('bench_events_topN_path_today_partial', function (string $queryId) use ($todayStart, $todayEnd): void {
181-
$this->adapter->setUseDailyRollups(true);
182121
$this->adapter->setNextQueryId($queryId);
183122
$this->usage->find([
184123
UsageQuery::groupBy('path'),
@@ -187,12 +126,9 @@ public function testBenchmarks(): void
187126
Query::lessThanEqual('time', $todayEnd),
188127
Query::limit(500),
189128
], Usage::TYPE_EVENT);
190-
$this->adapter->setUseDailyRollups(false);
191129
});
192130

193-
// Must route to raw — filter on a column the path MV doesn't index.
194131
$this->runBench('bench_events_topN_path_30d_filtered_resource', function (string $queryId) use ($start, $end): void {
195-
$this->adapter->setUseDailyRollups(true);
196132
$this->adapter->setNextQueryId($queryId);
197133
$this->usage->find([
198134
UsageQuery::groupBy('path'),
@@ -202,12 +138,9 @@ public function testBenchmarks(): void
202138
Query::lessThanEqual('time', $end),
203139
Query::limit(500),
204140
], Usage::TYPE_EVENT);
205-
$this->adapter->setUseDailyRollups(false);
206141
});
207142

208-
// Multi-dim breakdown — no single MV covers both path AND country.
209143
$this->runBench('bench_events_topN_path_country', function (string $queryId) use ($start, $end): void {
210-
$this->adapter->setUseDailyRollups(true);
211144
$this->adapter->setNextQueryId($queryId);
212145
$this->usage->find([
213146
UsageQuery::groupBy('path'),
@@ -217,7 +150,6 @@ public function testBenchmarks(): void
217150
Query::lessThanEqual('time', $end),
218151
Query::limit(500),
219152
], Usage::TYPE_EVENT);
220-
$this->adapter->setUseDailyRollups(false);
221153
});
222154

223155
// Write fan-out cost with all MVs attached. Compare against

‎tests/Benchmark/GaugesBench.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,6 @@ public function testBenchmarks(): void
3737
);
3838
});
3939

40-
$this->adapter->setUseDailyRollups(true);
41-
4240
$this->runBench('bench_gauges_topN_service_30d', function (string $queryId) use ($start30d, $endClosed): void {
4341
$this->adapter->setNextQueryId($queryId);
4442
$this->usage->find([
@@ -72,8 +70,6 @@ public function testBenchmarks(): void
7270
], Usage::TYPE_GAUGE);
7371
});
7472

75-
$this->adapter->setUseDailyRollups(false);
76-
7773
$this->assertNotEmpty($this->results, 'Benchmark scenarios must record results');
7874
}
7975
}

‎tests/Usage/Adapter/ClickHouseDimRoutingTest.php‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,6 @@ private function seedHistoricalRow(string $metric, int $value, string $modifier,
103103

104104
public function testTopNByPathRoutesToPathMv(): void
105105
{
106-
$this->adapter->setUseDailyRollups(true);
107106
$this->adapter->clearRouteLog();
108107

109108
$start = (new \DateTime('-7 days'))->format('Y-m-d H:i:s');
@@ -126,7 +125,6 @@ public function testTopNByPathRoutesToPathMv(): void
126125

127126
public function testTopNByCountryRoutesToCountryMv(): void
128127
{
129-
$this->adapter->setUseDailyRollups(true);
130128
$this->adapter->clearRouteLog();
131129

132130
$start = (new \DateTime('-7 days'))->format('Y-m-d H:i:s');
@@ -149,7 +147,6 @@ public function testTopNByCountryRoutesToCountryMv(): void
149147

150148
public function testTopNByServiceRoutesToServiceMv(): void
151149
{
152-
$this->adapter->setUseDailyRollups(true);
153150
$this->adapter->clearRouteLog();
154151

155152
$start = (new \DateTime('-7 days'))->format('Y-m-d H:i:s');
@@ -172,7 +169,6 @@ public function testTopNByServiceRoutesToServiceMv(): void
172169

173170
public function testTopNByMethodStatusRoutesToCombinedMv(): void
174171
{
175-
$this->adapter->setUseDailyRollups(true);
176172
$this->adapter->clearRouteLog();
177173

178174
$start = (new \DateTime('-7 days'))->format('Y-m-d H:i:s');
@@ -196,7 +192,6 @@ public function testTopNByMethodStatusRoutesToCombinedMv(): void
196192

197193
public function testMultiDimNotInAnySingleMvFallsBackToRaw(): void
198194
{
199-
$this->adapter->setUseDailyRollups(true);
200195
$this->adapter->clearRouteLog();
201196

202197
$this->usage->find([
@@ -214,7 +209,6 @@ public function testMultiDimNotInAnySingleMvFallsBackToRaw(): void
214209

215210
public function testFilterOnNonMvColumnFallsBackToRaw(): void
216211
{
217-
$this->adapter->setUseDailyRollups(true);
218212
$this->adapter->clearRouteLog();
219213

220214
$this->usage->find([
@@ -232,7 +226,6 @@ public function testFilterOnNonMvColumnFallsBackToRaw(): void
232226

233227
public function testSubDayIntervalForcesRaw(): void
234228
{
235-
$this->adapter->setUseDailyRollups(true);
236229
$this->adapter->clearRouteLog();
237230

238231
$this->usage->find([
@@ -250,7 +243,6 @@ public function testSubDayIntervalForcesRaw(): void
250243

251244
public function testWindowStraddlesTodayUsesHybridDim(): void
252245
{
253-
$this->adapter->setUseDailyRollups(true);
254246
$this->adapter->clearRouteLog();
255247

256248
$start = (new \DateTime('-7 days'))->format('Y-m-d H:i:s');
@@ -273,7 +265,6 @@ public function testWindowStraddlesTodayUsesHybridDim(): void
273265

274266
public function testDualReadSamplerActivates(): void
275267
{
276-
$this->adapter->setUseDailyRollups(true);
277268
$this->adapter->setDualReadSampleRate(1.0);
278269
$this->adapter->clearRouteLog();
279270

@@ -299,16 +290,16 @@ public function testDualReadSamplerActivates(): void
299290

300291
private function rawTotal(string $start, string $end): int
301292
{
302-
$this->adapter->setUseDailyRollups(false);
303-
$this->adapter->clearRouteLog();
304-
$sum = $this->usage->sum([
293+
$reflection = new ReflectionClass($this->adapter);
294+
$sumFromTable = $reflection->getMethod('sumFromTable');
295+
$sumFromTable->setAccessible(true);
296+
$result = $sumFromTable->invoke($this->adapter, [
305297
Query::equal('metric', [$this->metric]),
306298
Query::greaterThanEqual('time', $start),
307299
Query::lessThanEqual('time', $end),
308300
], 'value', Usage::TYPE_EVENT);
309-
$this->adapter->setUseDailyRollups(true);
310301
$this->adapter->clearRouteLog();
311-
return $sum;
302+
return is_int($result) ? $result : 0;
312303
}
313304

314305
/**

0 commit comments

Comments
 (0)