yorkie-team / yorkie

Yorkie is a document store for collaborative applications.
https://yorkie.dev
Apache License 2.0
780 stars 144 forks source link

Duplicate changes pushes during simultaneous sync and detach calls #895

Closed chacha912 closed 3 months ago

chacha912 commented 3 months ago

What happened:

When sync and detach functions are called simultaneously, local changes are being pushed to the server multiple times. Although server has logic to filter out already pushed changes, this logic doesn't work properly during the detach process.

During DetachDocument on the server side, the clientSeq and serverSeq of clientDocInfo are reset to 0 before pushing the changes, causing duplicate changes to be pushed.

What you expected to happen:

Local changes should only be pushed to the server once, even if sync and detach are called simultaneously.

How to reproduce it (as minimally and precisely as possible):

You can test this behavior by running the following test code in the js-sdk:

it.only('Should work correctly', async function ({
  task,
}) {
  type TestDoc = { k1: Array<number> };
  const docKey = toDocKey(`${task.name}-${new Date().getTime()}`);

  // 01. c1 attaches d1 and c2 watches same doc.
  const c1 = new yorkie.Client(testRPCAddr);
  await c1.activate();
  const d1 = new yorkie.Document<TestDoc>(docKey);
  d1.update((root) => {
    root['k1'] = [1, 2];
  }, 'set array');
  await c1.attach(d1, { syncMode: SyncMode.Manual });
  assert.equal(d1.toSortedJSON(), '{"k1":[1,2]}');

  const c2 = new yorkie.Client(testRPCAddr);
  await c2.activate();
  const d2 = new yorkie.Document<TestDoc>(docKey);
  await c2.attach(d2, { syncMode: SyncMode.Manual });
  assert.equal(d2.toSortedJSON(), '{"k1":[1,2]}');

  // 02. c1 updates d1.
  d1.update((root) => {
    root['k1'].push(3);
  });
  assert.equal(d1.toSortedJSON(), '{"k1":[1,2,3]}', 'd1');
  c1.sync();
  await c1.detach(d1);

  // 03. c2 syncs.
  await c2.sync();
  assert.equal(d2.toSortedJSON(), '{"k1":[1,2,3]}', 'd2');

  await c1.deactivate();
  await c2.deactivate();
});

Anything else we need to know?:

Environment: