perf: Skip the projection pass for merged deletes nothing visible outlives
A merge that deleted anything ran the full projection pass, whose closure follows foreign keys both ways and so loads every row connected to the deleted ones. A device deleting a card with its revisions, tags and reviews cost each device receiving it about 4 s, and the server merging the push about 5.4 s, in a library of 8,000 cards whose tags join it. A device deleting a row locally tombstones everything visible below it, so its batch carries those deletes along. What the pass could still decide is a row below that the deleting device never saw, inserted here meanwhile, which it hides by cascade. A batch that only deletes (none restoring) now leaves the pass out when the deleted tables cascade only, by the same rules as a local delete (_deletesOnlyCascade, shared with canSkipProjectionAfterLocalDelete now), no attempted value is pending in the connected tables, and every visible row below a deleted one, found by walking down the cascades alone, is deleted by the same batch (canLeaveProjectionUnchangedByMergedDeletes). A batch both inserting and deleting, or restoring, takes the pass as before. Measured on PostgreSQL: such a card delete merges in about 35 ms instead of 5.4 s; received on a device, 99 ms instead of 4 s. The new tests cover a leaf deleted through the shortcut and a middle row's delete, which isn't eligible, hiding its children by the pass; in the app's sync tests a card deleted while another device rated it declines the shortcut, and dropping the walk's check fails that test. The core, server, integration and simulation suites pass, but for two PostgreSQL tests that can't start their database while another embedded PostgreSQL runs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
f062725a16
commit
012d94d0ca
3 changed files with 205 additions and 14 deletions
|
|
@ -417,10 +417,13 @@ extension CrdtMergeRecorderExtension on CrdtMutationRecorder {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Whether this batch leaves every projection decision as it is though it
|
/// Whether this batch leaves every projection decision as it is though it
|
||||||
/// touches foreign keys: it only inserts new rows, each once, and updates
|
/// touches foreign keys, besides updating columns no projection reads: it
|
||||||
/// columns no projection reads, and the inserts are proven
|
/// only inserts new rows, each once, proven
|
||||||
/// ([CrdtForeignKeyProjector.canLeaveProjectionUnchangedByMergedInserts]).
|
/// ([CrdtForeignKeyProjector.canLeaveProjectionUnchangedByMergedInserts]),
|
||||||
/// A delete, or an insert onto a row there already, takes the full pass.
|
/// or only deletes rows, none restoring one, proven
|
||||||
|
/// ([CrdtForeignKeyProjector.canLeaveProjectionUnchangedByMergedDeletes]).
|
||||||
|
/// A batch doing both, an insert onto a row there already, or a restore
|
||||||
|
/// takes the full pass.
|
||||||
Future<bool> _mergeLeavesProjectionUnchanged(
|
Future<bool> _mergeLeavesProjectionUnchanged(
|
||||||
List<CrdtMergeChange> operations,
|
List<CrdtMergeChange> operations,
|
||||||
MergeContext context,
|
MergeContext context,
|
||||||
|
|
@ -428,11 +431,14 @@ extension CrdtMergeRecorderExtension on CrdtMutationRecorder {
|
||||||
) async {
|
) async {
|
||||||
final insertsByTable = <String, Map<UuidValue, Map<String, Object?>>>{};
|
final insertsByTable = <String, Map<UuidValue, Map<String, Object?>>>{};
|
||||||
final positions = <MergeRowKey, int>{};
|
final positions = <MergeRowKey, int>{};
|
||||||
|
final deletesByTable = <String, Set<UuidValue>>{};
|
||||||
for (final operation in operations) {
|
for (final operation in operations) {
|
||||||
if (!_context.isCrdtTrackedTableName(operation.tableName)) continue;
|
if (!_context.isCrdtTrackedTableName(operation.tableName)) continue;
|
||||||
switch (operation) {
|
switch (operation) {
|
||||||
case CrdtMergeDelete():
|
case final CrdtMergeDelete delete:
|
||||||
return false;
|
// An even flag deletes, an odd one restores.
|
||||||
|
if (delete.clFlag.isOdd || !delete.reason.isSynced) return false;
|
||||||
|
(deletesByTable[delete.tableName] ??= {}).add(delete.uuidRowId);
|
||||||
case final CrdtMergeUpdate update:
|
case final CrdtMergeUpdate update:
|
||||||
if (_foreignKeyProjector.needsProjection(update.tableName, {
|
if (_foreignKeyProjector.needsProjection(update.tableName, {
|
||||||
update.columnName,
|
update.columnName,
|
||||||
|
|
@ -452,6 +458,13 @@ extension CrdtMergeRecorderExtension on CrdtMutationRecorder {
|
||||||
positions[(insert.tableName, insert.uuidRowId)] = positions.length;
|
positions[(insert.tableName, insert.uuidRowId)] = positions.length;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if (insertsByTable.isNotEmpty && deletesByTable.isNotEmpty) return false;
|
||||||
|
if (deletesByTable.isNotEmpty) {
|
||||||
|
return _foreignKeyProjector.canLeaveProjectionUnchangedByMergedDeletes(
|
||||||
|
deletesByTable,
|
||||||
|
transaction,
|
||||||
|
);
|
||||||
|
}
|
||||||
return _foreignKeyProjector.canLeaveProjectionUnchangedByMergedInserts(
|
return _foreignKeyProjector.canLeaveProjectionUnchangedByMergedInserts(
|
||||||
insertsByTable,
|
insertsByTable,
|
||||||
positions,
|
positions,
|
||||||
|
|
|
||||||
|
|
@ -862,7 +862,63 @@ class CrdtForeignKeyProjector {
|
||||||
Future<bool> canSkipProjectionAfterLocalDelete(
|
Future<bool> canSkipProjectionAfterLocalDelete(
|
||||||
String tableName,
|
String tableName,
|
||||||
Transaction transaction,
|
Transaction transaction,
|
||||||
|
) async =>
|
||||||
|
_deletesOnlyCascade(tableName) &&
|
||||||
|
!await _hasAttemptedClaimsConnectedTo({tableName}, transaction);
|
||||||
|
|
||||||
|
/// Proves that merged deletes of [deletedByTable] (row ids by table, each
|
||||||
|
/// one deleting, none restoring) leave nothing for the projection pass to
|
||||||
|
/// decide. A device deleting a row locally tombstones everything visible
|
||||||
|
/// below it too ([canSkipProjectionAfterLocalDelete]), so its batch carries
|
||||||
|
/// those deletes along; what the pass could still decide is a row below
|
||||||
|
/// that the deleting device never saw, inserted here meanwhile, which it
|
||||||
|
/// hides by cascade. So, beyond the tables' own rules, every visible row
|
||||||
|
/// below a deleted one must be deleted by the same batch, found by
|
||||||
|
/// walking down the cascades alone, where the pass walks both ways through
|
||||||
|
/// everything connected.
|
||||||
|
Future<bool> canLeaveProjectionUnchangedByMergedDeletes(
|
||||||
|
Map<String, Set<UuidValue>> deletedByTable,
|
||||||
|
Transaction transaction,
|
||||||
) async {
|
) async {
|
||||||
|
if (!deletedByTable.keys.every(_deletesOnlyCascade)) return false;
|
||||||
|
if (await _hasAttemptedClaimsConnectedTo(
|
||||||
|
deletedByTable.keys.toSet(),
|
||||||
|
transaction,
|
||||||
|
)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
final deleted = {
|
||||||
|
for (final MapEntry(key: table, value: ids) in deletedByTable.entries)
|
||||||
|
for (final id in ids) (table, id),
|
||||||
|
};
|
||||||
|
var frontier = deletedByTable;
|
||||||
|
while (frontier.isNotEmpty) {
|
||||||
|
final next = <String, Set<UuidValue>>{};
|
||||||
|
for (final MapEntry(key: table, value: ids) in frontier.entries) {
|
||||||
|
if (ids.isEmpty) continue;
|
||||||
|
for (final edge
|
||||||
|
in _foreignKeys.edgesByParentTable[table] ?? const <ForeignKeyEdge>[]) {
|
||||||
|
for (final child in await _context.findVisibleDomainRowIdsWhere(
|
||||||
|
tableName: edge.childTableName,
|
||||||
|
predicates: [domainColumnInPredicate(edge.childColumn, ids)],
|
||||||
|
transaction: transaction,
|
||||||
|
)) {
|
||||||
|
if (!deleted.contains((edge.childTableName, child))) return false;
|
||||||
|
(next[edge.childTableName] ??= {}).add(child);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
frontier = next;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Whether a delete of [tableName] rows can only cascade: every table the
|
||||||
|
/// delete can reach is reached by cascades on id alone (no SET NULL or SET
|
||||||
|
/// DEFAULT repairs), none restricts its parent (a deleted row could be the
|
||||||
|
/// blocker of a withheld parent delete), and none has a unique index (a
|
||||||
|
/// released claim could restore a contestant) or a SET DEFAULT dependency.
|
||||||
|
bool _deletesOnlyCascade(String tableName) {
|
||||||
final cascadeTables = <String>{};
|
final cascadeTables = <String>{};
|
||||||
final queue = [tableName];
|
final queue = [tableName];
|
||||||
while (queue.isNotEmpty) {
|
while (queue.isNotEmpty) {
|
||||||
|
|
@ -887,16 +943,21 @@ class CrdtForeignKeyProjector {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
final reachable = _foreignKeys
|
return true;
|
||||||
.connectedTables({tableName})
|
|
||||||
.where(_context.syncTableByName.containsKey)
|
|
||||||
.toSet();
|
|
||||||
return !await _ProjectionReader(
|
|
||||||
_context,
|
|
||||||
transaction,
|
|
||||||
).hasAttemptedClaims(reachable);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Whether an attempted value is pending in a table connected to [tables],
|
||||||
|
/// which the pass would load and could resolve.
|
||||||
|
Future<bool> _hasAttemptedClaimsConnectedTo(
|
||||||
|
Set<String> tables,
|
||||||
|
Transaction transaction,
|
||||||
|
) => _ProjectionReader(_context, transaction).hasAttemptedClaims(
|
||||||
|
_foreignKeys
|
||||||
|
.connectedTables(tables)
|
||||||
|
.where(_context.syncTableByName.containsKey)
|
||||||
|
.toSet(),
|
||||||
|
);
|
||||||
|
|
||||||
/// Whether a mutation of [tableName] may require FK or unique projection.
|
/// Whether a mutation of [tableName] may require FK or unique projection.
|
||||||
bool needsProjection(String tableName, Set<String>? columnNames) {
|
bool needsProjection(String tableName, Set<String>? columnNames) {
|
||||||
if (_foreignKeys.columnsMayAffectForeignKeys(tableName, columnNames)) {
|
if (_foreignKeys.columnsMayAffectForeignKeys(tableName, columnNames)) {
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,117 @@
|
||||||
|
import 'package:serverpod_offline_sync_server/serverpod_offline_sync_server.dart';
|
||||||
|
import 'package:serverpod_offline_sync_test_client/serverpod_offline_sync_test_client.dart';
|
||||||
|
import 'package:test/test.dart';
|
||||||
|
|
||||||
|
import '../test_tools/client_session.dart';
|
||||||
|
import '../test_tools/crdt_probes.dart';
|
||||||
|
|
||||||
|
// Merged deletes of rows below which nothing visible stays leave the
|
||||||
|
// projection pass out; anything else takes it.
|
||||||
|
void main() {
|
||||||
|
initTestClientSession();
|
||||||
|
|
||||||
|
Future<void> merge(List<CrdtMergeChange> changes) =>
|
||||||
|
session.db.mergeChanges(changes, spaceId: testCrdtUserId);
|
||||||
|
|
||||||
|
Future<CrdtMergeDelete> deleteOf(String tableName, UuidValue rowId) async =>
|
||||||
|
CrdtMergeDelete(
|
||||||
|
uuidSpaceId: testCrdtUserId,
|
||||||
|
tableName: tableName,
|
||||||
|
uuidRowId: rowId,
|
||||||
|
uuidNodeId: const Uuid().v7obj(),
|
||||||
|
hlcDatetime: (await rowHlc(
|
||||||
|
rowId,
|
||||||
|
)).datetime.add(const Duration(milliseconds: 1)),
|
||||||
|
hlcCounter: 0,
|
||||||
|
clFlag: 2,
|
||||||
|
reason: CrdtDataDeletedReason.userDelete,
|
||||||
|
);
|
||||||
|
|
||||||
|
group('Given a set-null middle with two cascade children stored here,', () {
|
||||||
|
late FkChainSetNullMiddle middle;
|
||||||
|
late FkChainSetNullCascadeChild deleted;
|
||||||
|
late FkChainSetNullCascadeChild sibling;
|
||||||
|
|
||||||
|
setUp(() async {
|
||||||
|
await session.db.transactionForUser(testCrdtUserId, (tx) async {
|
||||||
|
middle = await FkChainSetNullMiddle.db.insertRow(
|
||||||
|
session,
|
||||||
|
FkChainSetNullMiddle(id: const Uuid().v7obj(), name: 'middle'),
|
||||||
|
transaction: tx,
|
||||||
|
);
|
||||||
|
deleted = await FkChainSetNullCascadeChild.db.insertRow(
|
||||||
|
session,
|
||||||
|
FkChainSetNullCascadeChild(
|
||||||
|
id: const Uuid().v7obj(),
|
||||||
|
name: 'deleted',
|
||||||
|
setNullMiddleId: middle.id,
|
||||||
|
),
|
||||||
|
transaction: tx,
|
||||||
|
);
|
||||||
|
sibling = await FkChainSetNullCascadeChild.db.insertRow(
|
||||||
|
session,
|
||||||
|
FkChainSetNullCascadeChild(
|
||||||
|
id: const Uuid().v7obj(),
|
||||||
|
name: 'sibling',
|
||||||
|
setNullMiddleId: middle.id,
|
||||||
|
),
|
||||||
|
transaction: tx,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
group('when a remote delete of one child is merged,', () {
|
||||||
|
setUp(() async {
|
||||||
|
await merge([
|
||||||
|
await deleteOf(FkChainSetNullCascadeChild.t.tableName, deleted.id!),
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('then only that child is hidden, by its delete.', () async {
|
||||||
|
expect(
|
||||||
|
await FkChainSetNullCascadeChild.db.findById(session, deleted.id!),
|
||||||
|
isNull,
|
||||||
|
);
|
||||||
|
expect(
|
||||||
|
await FkChainSetNullCascadeChild.db.findById(session, sibling.id!),
|
||||||
|
isNotNull,
|
||||||
|
);
|
||||||
|
expect(
|
||||||
|
await FkChainSetNullMiddle.db.findById(session, middle.id!),
|
||||||
|
isNotNull,
|
||||||
|
);
|
||||||
|
final row = await CrdtDataRow.db.findFirstRow(
|
||||||
|
session,
|
||||||
|
where: (t) => t.uuidRowId.equals(deleted.id),
|
||||||
|
);
|
||||||
|
expect(row!.visibility, CrdtDataRowVisibility.userDelete);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
group('when a remote delete of the middle alone is merged,', () {
|
||||||
|
setUp(() async {
|
||||||
|
await merge([
|
||||||
|
await deleteOf(FkChainSetNullMiddle.t.tableName, middle.id!),
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('then its children are hidden with it, by the cascade.', () async {
|
||||||
|
expect(
|
||||||
|
await FkChainSetNullMiddle.db.findById(session, middle.id!),
|
||||||
|
isNull,
|
||||||
|
);
|
||||||
|
for (final child in [deleted, sibling]) {
|
||||||
|
expect(
|
||||||
|
await FkChainSetNullCascadeChild.db.findById(session, child.id!),
|
||||||
|
isNull,
|
||||||
|
);
|
||||||
|
final row = await CrdtDataRow.db.findFirstRow(
|
||||||
|
session,
|
||||||
|
where: (t) => t.uuidRowId.equals(child.id),
|
||||||
|
);
|
||||||
|
expect(row!.visibility, CrdtDataRowVisibility.foreignKeyCascade);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
}
|
||||||
Loading…
Reference in a new issue