Skip to content

Commit 5754d98

Browse files
committed
WIP
1 parent 5f7d135 commit 5754d98

4 files changed

Lines changed: 150 additions & 14 deletions

File tree

src/Cache/Engine.php

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -220,13 +220,11 @@ public function select(
220220
}
221221

222222
try {
223-
$after = $this->states->resolve($context->plan, $context->namespace, $queryHash);
224-
225-
if ($after->equals($cached->state)) {
226-
$this->publish($context, $cached->state, $rows, $lease);
227-
} else {
228-
$this->leases->release($lease);
229-
}
223+
// publish_canonical and publish_versioned_entries both re-read the version
224+
// (and generation) inside the script and release the lease on a mismatch, so
225+
// a PHP-side re-check costs a round trip to repeat a weaker test: it cannot
226+
// close the window between reading the state and writing the entry.
227+
$this->publish($context, $cached->state, $rows, $lease);
230228
} catch (\Throwable $exception) {
231229
$this->leases->release($lease);
232230
$this->runtime->fail($exception);

src/Cache/MembershipRevalidator.php

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,16 @@
1313
/**
1414
* Every version in the gap costs one key in the change-record read, and change
1515
* records live as long as the entries they serve, so an uncapped gap on a hot table
16-
* would build an MGET of however many writes fit in one TTL. A successful
17-
* revalidation re-stamps the membership at the current version, so an actively read
18-
* query sits at a gap of one or two and never approaches this.
16+
* would build an MGET of however many writes fit in one TTL.
17+
*
18+
* The floor is set by read shape, not by write rate: a query re-read once per
19+
* rotation sits at a gap of the rotation depth times the table's writes per pass,
20+
* so a 20-page rotation over a table written once per request never fits under 20.
21+
* Measured on a 50/50 read/write benchmark, anything from 64 up performs the same
22+
* (10.5 vs 9.3 SQL selects per iteration at 64 vs 512, wall time flat); 8 rejected
23+
* 49% of otherwise revalidatable reads.
1924
*/
20-
private const MAX_VERSION_GAP = 8;
25+
private const MAX_VERSION_GAP = 128;
2126

2227
public function __construct(
2328
private RedisStore $store,

tests/Integration/Cache/MembershipRevalidationTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ protected function setUp(): void
2727
}
2828

2929
/** Mirrors MembershipRevalidator::MAX_VERSION_GAP. */
30-
private const MAX_VERSION_GAP = 8;
30+
private const MAX_VERSION_GAP = 128;
3131

3232
private function enableRevalidation(): void
3333
{
@@ -450,8 +450,8 @@ public function test_a_version_gap_beyond_the_cap_does_not_revalidate(): void
450450

451451
// The seed insert already consumed one version, so this leaves a gap of
452452
// MAX_VERSION_GAP + 1 between the membership and the current version.
453-
foreach (range(1, self::MAX_VERSION_GAP + 1) as $id) {
454-
RawPost::query()->toBase()->where('id', $id)->update(['title' => "t{$id}"]);
453+
foreach (range(1, self::MAX_VERSION_GAP + 1) as $pass) {
454+
RawPost::query()->toBase()->where('id', 7)->update(['title' => "t{$pass}"]);
455455
}
456456

457457
$this->assertNotRevalidated(fn() => RawPost::query()->toBase()->get());
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
<?php
2+
3+
namespace NormCache\Tests\Integration\Cache;
4+
5+
use Illuminate\Support\Facades\DB;
6+
use NormCache\Planning\TableIdentityResolver;
7+
use NormCache\Tests\Fixtures\Models\Author;
8+
use NormCache\Tests\Fixtures\Models\RawPost;
9+
use NormCache\Tests\TestCase;
10+
use NormCache\Values\TableIdentity;
11+
12+
/**
13+
* A write that lands while a miss is fetching from the database must not be overwritten
14+
* by the publish that follows it. The guard is inside publish_canonical /
15+
* publish_versioned_entries, so these cover it without a PHP-side pre-check.
16+
*/
17+
final class PublishRaceTest extends TestCase
18+
{
19+
private int $authorId;
20+
21+
protected function setUp(): void
22+
{
23+
parent::setUp();
24+
25+
$this->authorId = (int) Author::query()->create(['name' => 'Author'])->getKey();
26+
27+
$rows = [];
28+
29+
for ($index = 1; $index <= 20; $index++) {
30+
$rows[] = [
31+
'id' => $index,
32+
'title' => "Post {$index}",
33+
'views' => $index,
34+
'published' => true,
35+
'author_id' => $this->authorId,
36+
'created_at' => now(),
37+
'updated_at' => now(),
38+
];
39+
}
40+
41+
RawPost::query()->toBase()->insert($rows);
42+
}
43+
44+
private function tableIdentity(string $table): TableIdentity
45+
{
46+
$identity = $this->app->make(TableIdentityResolver::class)
47+
->resolve(DB::connection(), $table);
48+
49+
$this->assertNotNull($identity);
50+
51+
return $identity;
52+
}
53+
54+
/** Bumps the table version once, while the given select is in flight. */
55+
private function bumpVersionDuring(string $sqlNeedle, callable $callback): mixed
56+
{
57+
$bumped = false;
58+
$versionKey = $this->cacheKeys()->version($this->tableIdentity('posts'));
59+
60+
DB::listen(function ($query) use (&$bumped, $sqlNeedle, $versionKey): void {
61+
if ($bumped || !str_contains($query->sql, $sqlNeedle)) {
62+
return;
63+
}
64+
65+
$bumped = true;
66+
$this->cacheStore()->increment($versionKey);
67+
});
68+
69+
$result = $callback();
70+
71+
$this->assertTrue($bumped, 'expected the miss to reach the database');
72+
73+
return $result;
74+
}
75+
76+
public function test_a_version_bump_during_the_build_leaves_no_canonical_entry(): void
77+
{
78+
$rows = $this->bumpVersionDuring(
79+
'select * from "posts"',
80+
fn() => RawPost::query()->toBase()->get(),
81+
);
82+
83+
$this->assertCount(20, $rows, 'the caller still gets its rows');
84+
$this->assertSame(
85+
[],
86+
$this->cacheQueryKeysWithField('m'),
87+
'the membership must not be published against a version that already moved',
88+
);
89+
}
90+
91+
public function test_the_read_after_a_raced_build_is_a_miss_not_stale_data(): void
92+
{
93+
$this->bumpVersionDuring(
94+
'select * from "posts"',
95+
fn() => RawPost::query()->toBase()->get(),
96+
);
97+
98+
RawPost::query()->toBase()->where('id', 3)->update(['title' => 'changed']);
99+
100+
$rows = collect(RawPost::query()->toBase()->get());
101+
102+
$this->assertSame('changed', $rows->firstWhere('id', 3)->title);
103+
}
104+
105+
public function test_a_version_bump_during_a_result_build_leaves_no_entry(): void
106+
{
107+
$count = $this->bumpVersionDuring(
108+
'select count(*)',
109+
fn() => RawPost::query()->toBase()->count(),
110+
);
111+
112+
$this->assertSame(20, $count);
113+
$this->assertSame(
114+
[],
115+
$this->cacheQueryKeysWithField('r'),
116+
'the result payload must not be published against a moved version',
117+
);
118+
}
119+
120+
public function test_the_lease_is_released_when_the_publish_guard_rejects(): void
121+
{
122+
$this->bumpVersionDuring(
123+
'select * from "posts"',
124+
fn() => RawPost::query()->toBase()->get(),
125+
);
126+
127+
$this->assertSame(
128+
[],
129+
$this->cacheKeysMatching(':build:'),
130+
'a rejected publish must not leak the build lease',
131+
);
132+
}
133+
}

0 commit comments

Comments
 (0)