Skip to content

[ASI-756] PR #3 - Update snapback logic to consume filesHash - #2186

Merged
SidSethi merged 46 commits into
masterfrom
ss-new-snapback-sync-logic
Jan 28, 2022
Merged

[ASI-756] PR #3 - Update snapback logic to consume filesHash#2186
SidSethi merged 46 commits into
masterfrom
ss-new-snapback-sync-logic

Conversation

@SidSethi

@SidSethiSidSethi commented Dec 21, 2021

Copy link
Copy Markdown
Contributor

Description

  • Update Snapback issueSyncRequests logic to consume filesHash, with back-compat
  • New computeSyncModeForUserAndReplica module

Tests

  • unit test coverage for computeSyncModeForUserAndReplica()
  • Existing maddog test coverage + local e2e confirm no regression

How will this change be monitored?

Monitor recurring sync failures, and watch out for logs:

  • "[issueSyncRequestsToSecondaries] [PrimaryShouldSync = true] wallet ${wallet} secondary ${secondary} Clocks: [${primaryClock},${secondaryClock}] Files hashes: [${primaryFilesHash},${secondaryFilesHash}]"
  • "[issueSyncRequestsToSecondaries] Falling back to computeSyncModeForUserAndReplicaLegacy() [primaryFilesHash: ${primaryFilesHash}] secondaryFilesHash: ${secondaryFilesHash}"

Base automatically changed from ss-sec-wipe-resync to masterJanuary 5, 2022 15:35
@SidSethi
SidSethi changed the base branch from master to ss-sec-wipe-resyncJanuary 5, 2022 15:39
@SidSethi
SidSethi changed the base branch from ss-sec-wipe-resync to masterJanuary 5, 2022 15:53
Comment threadcreator-node/src/snapbackSM/snapbackSM.js Outdated
@SidSethi
SidSethi marked this pull request as ready for review January 5, 2022 19:09

@dmanjunathdmanjunath left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very clean and easy to understand

Comment threadcreator-node/src/snapbackSM/snapbackSM.js
Comment threadcreator-node/src/snapbackSM/snapbackSM.js Outdated
@vicky-g

Copy link
Copy Markdown
Contributor

might also be worth while to create loggly dashboards to parse for the logs mentioned in the bio

@SidSethi

Copy link
Copy Markdown
ContributorAuthor

might also be worth while to create loggly dashboards to parse for the logs mentioned in the bio

@vicky-g seems overkill - simple to query normally, can revisit after some time if needed

@SidSethi

Copy link
Copy Markdown
ContributorAuthor

@vicky-g@dmanjunath addressed all comments, couple outstanding discussions - pls re-review!!

@dmanjunathdmanjunath left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glad to see async-retry works with this!

@vicky-gvicky-g left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

almost there! just a small comment regarding creator-node/src/snapbackSM/computeSyncModeForUserAndReplica.js

Comment threadcreator-node/src/snapbackSM/computeSyncModeForUserAndReplica.js Outdated
Comment threadcreator-node/src/snapbackSM/computeSyncModeForUserAndReplica.js Outdated
@SidSethi
SidSethi requested a review from vicky-gJanuary 27, 2022 21:24

@vicky-gvicky-g left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@SidSethi@vicky-g@dmanjunath@cheran-senthil