Fix storage sync edge cases

This commit is contained in:
AsamK 2026-09-10 19:25:55 +02:00
parent ccd7c77a9d
commit ff689b577b
4 changed files with 41 additions and 3 deletions

View File

@ -414,7 +414,7 @@ public class StorageHelper {
final WriteOperationResult writeOperation, final WriteOperationResult writeOperation,
final Set<StorageId> identityConflictsPendingRepair final Set<StorageId> identityConflictsPendingRepair
) { ) {
return !writeOperation.inserts().isEmpty() && writeOperation.inserts() return writeOperation.deletes().isEmpty() && !writeOperation.inserts().isEmpty() && writeOperation.inserts()
.stream() .stream()
.allMatch(record -> identityConflictsPendingRepair.contains(record.getId())); .allMatch(record -> identityConflictsPendingRepair.contains(record.getId()));
} }

View File

@ -20,6 +20,8 @@ public final class StorageSyncLoopDetector {
private final LeakyBucket rateBucket = new LeakyBucket(100, private final LeakyBucket rateBucket = new LeakyBucket(100,
Duration.ofMinutes(10).toMillis(), Duration.ofMinutes(10).toMillis(),
new InMemoryBucketState()); new InMemoryBucketState());
private boolean lastAttemptChargedContent;
private boolean lastAttemptChargedRate;
public StorageSyncLoopDetector(final BooleanSupplier isMultiDevice) { public StorageSyncLoopDetector(final BooleanSupplier isMultiDevice) {
this.isMultiDevice = isMultiDevice; this.isMultiDevice = isMultiDevice;
@ -39,6 +41,8 @@ public final class StorageSyncLoopDetector {
final boolean isRetry, final boolean isRetry,
final long now final long now
) { ) {
lastAttemptChargedContent = false;
lastAttemptChargedRate = false;
if (!isMultiDevice.getAsBoolean() || isRetry) { if (!isMultiDevice.getAsBoolean() || isRetry) {
return Decision.Allowed.INSTANCE; return Decision.Allowed.INSTANCE;
} }
@ -56,9 +60,11 @@ public final class StorageSyncLoopDetector {
if (chargeContent) { if (chargeContent) {
contentBucket.use(now); contentBucket.use(now);
lastAttemptChargedContent = true;
} }
if (fetchedRemoteManifest) { if (fetchedRemoteManifest) {
rateBucket.use(now); rateBucket.use(now);
lastAttemptChargedRate = true;
} }
if (fingerprint != null) { if (fingerprint != null) {
remember(fingerprint); remember(fingerprint);
@ -72,8 +78,14 @@ public final class StorageSyncLoopDetector {
} }
synchronized void onWriteFailed(final long now) { synchronized void onWriteFailed(final long now) {
contentBucket.refund(now); if (lastAttemptChargedContent) {
rateBucket.refund(now); contentBucket.refund(now);
lastAttemptChargedContent = false;
}
if (lastAttemptChargedRate) {
rateBucket.refund(now);
lastAttemptChargedRate = false;
}
} }
public synchronized void onConverged() { public synchronized void onConverged() {

View File

@ -38,6 +38,9 @@ class StorageHelperTest {
assertFalse(StorageHelper.containsOnlyIdentityConflictsPendingRepair(write(List.of(pendingRecord, otherRecord)), assertFalse(StorageHelper.containsOnlyIdentityConflictsPendingRepair(write(List.of(pendingRecord, otherRecord)),
Set.of(pendingId))); Set.of(pendingId)));
assertFalse(StorageHelper.containsOnlyIdentityConflictsPendingRepair(write(List.of()), Set.of(pendingId))); assertFalse(StorageHelper.containsOnlyIdentityConflictsPendingRepair(write(List.of()), Set.of(pendingId)));
assertFalse(StorageHelper.containsOnlyIdentityConflictsPendingRepair(new WriteOperationResult(null,
List.of(pendingRecord),
List.of(new byte[]{3})), Set.of(pendingId)));
} }
private static SignalStorageRecord record(final StorageId id) { private static SignalStorageRecord record(final StorageId id) {

View File

@ -65,6 +65,29 @@ class StorageSyncLoopDetectorTest {
} }
} }
@Test
void failedWriteRefundsOnlyBucketsChargedByLatestAttempt() {
final var contentDetector = new StorageSyncLoopDetector(() -> true);
final var repeatedWrite = writeWithInsert(1);
for (var index = 0; index < 4; index++) {
assertAllowed(contentDetector.onWriteAttempt(repeatedWrite, true, false, NOW));
}
final var deleteOnlyWrite = new WriteOperationResult(null, List.of(), List.of(new byte[]{1}));
assertAllowed(contentDetector.onWriteAttempt(deleteOnlyWrite, true, false, NOW));
contentDetector.onWriteFailed(NOW);
assertEquals(new StorageSyncLoopDetector.Decision.Denied(StorageSyncLoopDetector.Cause.REPEATED_PAYLOAD, 3),
contentDetector.onWriteAttempt(repeatedWrite, true, false, NOW));
final var rateDetector = new StorageSyncLoopDetector(() -> true);
for (var index = 0; index < 100; index++) {
assertAllowed(rateDetector.onWriteAttempt(deleteOnlyWrite, true, false, NOW));
}
assertAllowed(rateDetector.onWriteAttempt(deleteOnlyWrite, false, false, NOW));
rateDetector.onWriteFailed(NOW);
assertEquals(new StorageSyncLoopDetector.Decision.Denied(StorageSyncLoopDetector.Cause.WRITE_RATE, 100),
rateDetector.onWriteAttempt(deleteOnlyWrite, true, false, NOW));
}
@Test @Test
void rateBucketLimitsDeleteOnlyWrites() { void rateBucketLimitsDeleteOnlyWrites() {
final var detector = new StorageSyncLoopDetector(() -> true); final var detector = new StorageSyncLoopDetector(() -> true);