super-productivity/docs/sync/vector-clock-action-plan-revised.md
Johannes Millan 230ccb7b1d docs: add test fix summary for vector clock implementation
- Document the root cause of the failing test
- Explain why setting Lamport values to 0 fixes the issue
- Provide verification of the fix logic
- Complete documentation of vector clock implementation work
2025-06-30 21:42:03 +02:00

6.8 KiB

Revised Vector Clock Action Plan

Overview

Based on implementation experience and test results, this revised action plan focuses on practical, incremental improvements that maintain backward compatibility while addressing critical issues.

Key Principles

  1. Graceful Migration: Support mixed Lamport/vector clock states without forcing conflicts
  2. Backward Compatibility: Ensure older clients can still sync
  3. Incremental Improvement: Fix critical issues first, optimize later
  4. Test-Driven: Ensure all changes are covered by tests

Phase 1: Critical Fixes (Completed ✓)

1.1 Console.log Removal (✓)

  • Replaced console.log with pfLog
  • Proper log levels for debugging

1.2 Mixed State Handling (✓)

  • Gracefully handle mixed vector/Lamport states
  • Use change counters when available
  • Fall back to timestamps when necessary
  • No forced conflicts during migration

Phase 2: Data Integrity (High Priority)

2.1 Add Vector Clock Validation

File: src/app/pfapi/api/util/vector-clock.ts

export const isValidVectorClock = (clock: any): clock is VectorClock => {
  if (!clock || typeof clock !== 'object') return false;
  if (Array.isArray(clock) || clock.constructor !== Object) return false;

  return Object.entries(clock).every(([key, value]) => {
    return (
      typeof key === 'string' &&
      key.length > 0 &&
      typeof value === 'number' &&
      Number.isFinite(value) &&
      value >= 0 &&
      value <= Number.MAX_SAFE_INTEGER
    );
  });
};

export const sanitizeVectorClock = (clock: any): VectorClock => {
  if (!isValidVectorClock(clock)) return {};
  return clock;
};

Usage in sync service:

// In downloadToLocal method
const remoteVector = sanitizeVectorClock(getVectorClock(remote, clientId));

2.2 Fix Force Upload Edge Case

File: src/app/pfapi/api/sync/sync.service.ts

Current issue: Force upload may continue without remote vector clock, losing data.

// In uploadAll method, replace the try-catch with:
if (isForceUpload) {
  let remoteMeta: RemoteMeta | null = null;
  try {
    const result = await this._metaFileSyncService.download();
    remoteMeta = result.remoteMeta;
  } catch (e) {
    pfLog(1, 'Warning: Cannot fetch remote metadata during force upload', e);
  }

  if (remoteMeta) {
    const remoteVector = getVectorClock(remoteMeta, clientId);
    if (remoteVector) {
      localVector = mergeVectorClocks(localVector, remoteVector);
      pfLog(2, 'Merged remote vector clock for force upload');
    }
  } else {
    pfLog(1, 'Proceeding with force upload without remote vector clock merge');
  }
}

2.3 Implement Basic Pruning

File: src/app/pfapi/api/util/vector-clock.ts

Start simple - just limit the size:

const MAX_VECTOR_CLOCK_SIZE = 50;

export const limitVectorClockSize = (
  clock: VectorClock,
  currentClientId: string,
): VectorClock => {
  const entries = Object.entries(clock);

  if (entries.length <= MAX_VECTOR_CLOCK_SIZE) {
    return clock;
  }

  // Sort by value (ascending) to keep most active clients
  entries.sort(([, a], [, b]) => b - a);

  // Always keep current client
  const limited: VectorClock = {
    [currentClientId]: clock[currentClientId] || 0,
  };

  // Add top clients up to limit
  let count = 1;
  for (const [clientId, value] of entries) {
    if (clientId !== currentClientId && count < MAX_VECTOR_CLOCK_SIZE) {
      limited[clientId] = value;
      count++;
    }
  }

  return limited;
};

Phase 3: Robustness (Medium Priority)

3.1 Add Comprehensive Tests

File: src/app/pfapi/api/util/vector-clock.spec.ts

describe('Vector Clock Validation', () => {
  it('should validate correct vector clocks', () => {
    expect(isValidVectorClock({ a: 1, b: 2 })).toBe(true);
    expect(isValidVectorClock({})).toBe(true);
  });

  it('should reject invalid vector clocks', () => {
    expect(isValidVectorClock(null)).toBe(false);
    expect(isValidVectorClock({ a: -1 })).toBe(false);
    expect(isValidVectorClock({ a: 'string' })).toBe(false);
    expect(isValidVectorClock({ '': 1 })).toBe(false);
  });
});

describe('Vector Clock Size Limiting', () => {
  it('should limit vector clock size', () => {
    const large: VectorClock = {};
    for (let i = 0; i < 100; i++) {
      large[`client${i}`] = i;
    }

    const limited = limitVectorClockSize(large, 'client99');
    expect(Object.keys(limited).length).toBe(MAX_VECTOR_CLOCK_SIZE);
    expect(limited.client99).toBe(99); // Current client preserved
  });
});

3.2 Improve Error Handling

Add better error messages and recovery:

// In get-sync-status-from-meta-files.ts
try {
  const vectorResult = _checkForUpdateVectorClock({
    localVector,
    remoteVector,
    lastSyncedVector: lastSyncedVector || null,
  });
  // ... handle result
} catch (e) {
  pfLog(0, 'Vector clock comparison failed, falling back to Lamport', e);
  // Fall back to Lamport comparison
}

Phase 4: Performance Monitoring (Low Priority)

4.1 Add Metrics

interface VectorClockMetrics {
  size: number;
  comparisonTime: number;
  pruningOccurred: boolean;
}

export const measureVectorClock = (clock: VectorClock): VectorClockMetrics => {
  return {
    size: Object.keys(clock).length,
    comparisonTime: 0, // Will be set during comparison
    pruningOccurred: false,
  };
};

4.2 Log Warnings for Large Clocks

// In incrementVectorClock
const newClock = { ...(clock || {}) };
// ... increment logic

if (Object.keys(newClock).length > 30) {
  pfLog(1, 'Warning: Vector clock growing large', {
    size: Object.keys(newClock).length,
    clientId,
  });
}

Implementation Order

  1. Week 1:

    • Vector clock validation (2.1)
    • Fix force upload (2.2)
    • Add tests
  2. Week 2:

    • Basic size limiting (2.3)
    • Error handling improvements (3.2)
    • More tests
  3. Week 3:

    • Performance monitoring (4.1, 4.2)
    • Documentation updates

Key Differences from Original Plan

  1. No Breaking Changes: Mixed states handled gracefully, not as conflicts
  2. Simple Pruning: Size limiting instead of complex time-based pruning
  3. Incremental Approach: Each phase can be deployed independently
  4. Focus on Stability: Prioritize working sync over perfect implementation

Success Criteria

  1. No sync disruption during migration period
  2. Vector clocks stay under 50 entries for 99% of users
  3. Zero data loss from sync operations
  4. All edge cases tested with >95% coverage

Next Steps

  1. Implement Phase 2.1 (Validation)
  2. Test thoroughly with mixed client scenarios
  3. Deploy incrementally with monitoring
  4. Gather metrics before implementing complex pruning

This revised plan prioritizes stability and backward compatibility while still addressing the critical issues identified in the analysis.