Skip to content

Commit 5d4c123

Browse files
kriszypcodex
andcommitted
test(cache): remove conflict timing assumptions
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
1 parent eb598f8 commit 5d4c123

2 files changed

Lines changed: 21 additions & 16 deletions

File tree

resources/Table.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5650,6 +5650,18 @@ export function makeTable(options) {
56505650
const metadataFlags = existingEntry?.metadataFlags;
56515651

56525652
const existingVersion = existingEntry?.version;
5653+
const existingRecord = existingEntry?.value;
5654+
const inheritedTimestamp = context?.timestamp || context?.transaction?.timestamp;
5655+
const monotonicTimestamp = () =>
5656+
isRocksDB ? (primaryStore as RocksDatabase).getMonotonicTimestamp() : getNextMonotonicTime();
5657+
const nextExistingVersion =
5658+
existingVersion == null
5659+
? 0
5660+
: existingVersion + Math.max(Math.abs(existingVersion) * Number.EPSILON, Number.MIN_VALUE);
5661+
const sourceTimestamp =
5662+
inheritedTimestamp && (existingVersion == null || inheritedTimestamp > existingVersion)
5663+
? inheritedTimestamp
5664+
: Math.max(monotonicTimestamp(), nextExistingVersion);
56535665
let whenResolved, timer;
56545666
// We start by locking the record so that there is only one resolution happening at once;
56555667
// if there is already a resolution in process, we want to use the results of that resolution
@@ -5688,17 +5700,9 @@ export function makeTable(options) {
56885700
// lock acquired — this request will actually load from source
56895701
setLoadedFromSource(target, true);
56905702

5691-
const existingRecord = existingEntry?.value;
5692-
const inheritedTimestamp = context?.timestamp || context?.transaction?.timestamp;
5693-
const monotonicTimestamp = () =>
5694-
isRocksDB ? (primaryStore as RocksDatabase).getMonotonicTimestamp() : getNextMonotonicTime();
5695-
const sourceTimestamp =
5696-
inheritedTimestamp && (existingVersion == null || inheritedTimestamp > existingVersion)
5697-
? inheritedTimestamp
5698-
: Math.max(monotonicTimestamp(), existingVersion == null ? 0 : existingVersion + 0.000488);
56995703
// it is important to remember that this is _NOT_ part of the current transaction; nothing is changing
5700-
// with the canonical data, we are simply fulfilling our local copy of the canonical data, but still don't
5701-
// want a timestamp later than the current transaction
5704+
// with the canonical data, we are simply fulfilling our local copy of the canonical data. We preserve the
5705+
// request timestamp unless advancing it is required to replace an existing version.
57025706
// we create a new context for the source, we want to determine the timestamp and don't want to
57035707
// attribute this to the current user
57045708
const sourceContext = {

unitTests/resources/caching.test.js

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ describe('Caching', () => {
505505
await waitFor(() => conflictSourceRequest?.id === id);
506506
const sourceTimestamp = conflictSourceRequest.timestamp;
507507
assert.equal(typeof sourceTimestamp, 'number');
508-
await ConflictCachingTable.put(id, { id, name: 'authoritative' });
508+
await ConflictCachingTable.put(id, { id, name: 'authoritative' }, { timestamp: sourceTimestamp + 1 });
509509
await waitFor(() => ConflictCachingTable.primaryStore.getSync(id)?.name === 'authoritative');
510510
const authoritativeVersion = ConflictCachingTable.primaryStore.getEntry(id).version;
511511
conflictSourceRequest.respond({ id, name: 'source' });
@@ -523,15 +523,16 @@ describe('Caching', () => {
523523
const existingVersion = ConflictCachingTable.primaryStore.getEntry(id).version;
524524
await ConflictCachingTable.invalidate(id);
525525
await waitFor(() => ConflictCachingTable.primaryStore.getEntry(id)?.metadataFlags);
526+
const invalidatedVersion = ConflictCachingTable.primaryStore.getEntry(id).version;
526527
conflictSourceRequest = null;
527528
const fill = ConflictCachingTable.get(id, { timestamp: existingVersion - 1000 });
528529
await waitFor(() => conflictSourceRequest?.id === id);
529-
assert(conflictSourceRequest.timestamp > existingVersion);
530+
assert(conflictSourceRequest.timestamp > invalidatedVersion);
530531
conflictSourceRequest.respond({ id, name: 'refreshed' });
531532
assert.equal((await fill).name, 'refreshed');
532533
await waitFor(() => !ConflictCachingTable.primaryStore.hasLock(id));
533534
assert.equal(ConflictCachingTable.primaryStore.getSync(id).name, 'refreshed');
534-
assert(ConflictCachingTable.primaryStore.getEntry(id).version > existingVersion);
535+
assert(ConflictCachingTable.primaryStore.getEntry(id).version > invalidatedVersion);
535536
});
536537

537538
it('first source fill replaces an older raced record and its index entries', async function () {
@@ -541,7 +542,7 @@ describe('Caching', () => {
541542
await waitFor(() => conflictSourceRequest?.id === id);
542543
const sourceTimestamp = conflictSourceRequest.timestamp;
543544
const createdAt = new Date(sourceTimestamp - 0.0001);
544-
await ConflictCachingTable.put(id, { id, name: 'older', createdAt });
545+
await ConflictCachingTable.put(id, { id, name: 'older', createdAt }, { timestamp: sourceTimestamp - 1 });
545546
await waitFor(() => ConflictCachingTable.primaryStore.getSync(id)?.name === 'older');
546547
const racedEntry = ConflictCachingTable.primaryStore.getEntry(id);
547548
assert(racedEntry.version < sourceTimestamp);
@@ -574,7 +575,7 @@ describe('Caching', () => {
574575
const fill = ConflictCachingTable.get(id, { timestamp: nextConflictTimestamp() });
575576
await waitFor(() => conflictSourceRequest?.id === id);
576577
const sourceTimestamp = conflictSourceRequest.timestamp;
577-
await ConflictCachingTable.put(id, { id, name: 'raced' });
578+
await ConflictCachingTable.put(id, { id, name: 'raced' }, { timestamp: sourceTimestamp - 1 });
578579
await waitFor(() => ConflictCachingTable.primaryStore.getSync(id)?.name === 'raced');
579580
assert(ConflictCachingTable.primaryStore.getEntry(id).version < sourceTimestamp);
580581
const racedVersion = ConflictCachingTable.primaryStore.getEntry(id).version;
@@ -591,7 +592,7 @@ describe('Caching', () => {
591592
const fill = ConflictCachingTable.get(id, { timestamp: nextConflictTimestamp() });
592593
await waitFor(() => conflictSourceRequest?.id === id);
593594
const sourceTimestamp = conflictSourceRequest.timestamp;
594-
await ConflictCachingTable.put(id, { id, name: 'older-delete' });
595+
await ConflictCachingTable.put(id, { id, name: 'older-delete' }, { timestamp: sourceTimestamp - 1 });
595596
await waitFor(() => ConflictCachingTable.primaryStore.getSync(id)?.name === 'older-delete');
596597
assert(ConflictCachingTable.primaryStore.getEntry(id).version < sourceTimestamp);
597598
const racedVersion = ConflictCachingTable.primaryStore.getEntry(id).version;

0 commit comments

Comments
 (0)