SecondarySyncHealthTracker change recordfail #4353

vicky-g · 2022-11-15T18:14:45Z

Description

Summary: This PR is to relieve pressure on redis by only tracking the errors encountered on the primary->secondary syncs and adding fine tuned granularity on the error encountered to the number of times encountered.

This PR includes:

removing old env vars \ configs
adding a prometheus error
rewriting SecondarySyncHealthTracker to go from success rate \ error rate computation -> error count
changing how SecondarySyncHealthTracker is consumed around various places
updated relevant tests
wrote new tests (see Tests)

Tests

Unit tests via SecondarySyncHealthTracker.test.ts
Manual tests

Happy path:

Created a user
Uploaded a track
Checked redis + logs to see flow

Wallet was able to sync successfully with no errors recorded in redis

Redis

127.0.0.1:6379> keys PRIMARY_TO_SECONDARY_SYNC_FAILURE*
(empty list or set)
127.0.0.1:6379>

User state

User 2
wallet 0xa89416610a90a7655c432434f763f0bb7d9189ad
handle seed_221129mz8g
creator node endpoint http://cn1_creator-node_1:4000,http://cn3_creator-node_1:4002,http://cn2_creator-node_1:4001

The clocks:  {
  'http://cn1_creator-node_1:4000': 11,
  'http://cn2_creator-node_1:4001': 11,
  'http://cn3_creator-node_1:4002': 11,
  'http://cn4_creator-node_1:4003': -1
}

Sad path:

Forced update replica sets to fail (part of sync flow)
Created a user
Uploaded a track
Checked redis + logs to see flow

Uploaded track and user only had primary clock values increase:

User 1
wallet 0xbc13d58475359394a75795e292bbe4aeb254cf18
handle seed_22112958hj
creator node endpoint http://cn3_creator-node_1:4002,http://cn2_creator-node_1:4001,http://cn4_creator-node_1:4003

The clocks:  {
  'http://cn1_creator-node_1:4000': -1,
  'http://cn2_creator-node_1:4001': 20,
  'http://cn3_creator-node_1:4002': 29,
  'http://cn4_creator-node_1:4003': 20
}

SecondarySyncHealthTracker logs highlighting wallet encountered max errors allowed:

{"name":"audius_creator_node","clusterWorker":"Worker 1","hostname":"51f7210dfa94","pid":134,"level":40,"SecondarySyncHealthTracker":"computeWalletOnSecondaryExceedsMaxErrorsAllowed","msg":"Wallets on secondaries have exceeded the allowed error capacity for today: {\"0xbc13d58475359394a75795e292bbe4aeb254cf18\":{\"http://cn2_creator-node_1:4001\":\"failure_fetching_user_replica_set\",\"http://cn4_creator-node_1:4003\":\"failure_fetching_user_replica_set\"}}","time":"2022-11-29T17:26:56.452Z","v":0,"trace_id":"4ab33a571b7d03cb1f24fc9d503a80a8","span_id":"4e7bf002cb84f1ab","trace_flags":"01","resource.span.name":"issueSyncRequest.jobProcessor","resource.span.links":[{"context":{"traceId":"e8a4ceb28bfb672e51649a6406444db9","spanId":"395c9ea02aa69ce4","traceFlags":1},"attributes":{}}],"resource.span.attributed":{"code.filepath":"/usr/src/app/src/services/stateMachineManager/stateReconciliation/issueSyncRequest.jobProcessor.ts","code.function":"issueSyncRequest","spid":0,"endpoint":"http://cn3_creator-node_1:4002"},"resource.service.name":"content-node","resource.service.spid":0,"resource.service.endpoint":"http://cn3_creator-node_1:4002","logLevel":"warn"}

and

{"name":"audius_creator_node","clusterWorker":"master","hostname":"5b39db87d2af","pid":20708,"level":40,"SecondarySyncHealthTracker":"doesWalletOnSecondaryExceedMaxErrorsAllowed","wallet":"0xc3d1d7559356aac64ba2921ec266ce294dcdc981","secondary":"http://cn4_creator-node_1:4003","error":"failure_fetching_user_replica_set","msg":"Wallet encountered max errors allowed on secondary","time":"2022-11-30T01:52:17.790Z","v":0,"trace_id":"849f74275a833943fe862c41772b27a6","span_id":"5bbb5776175182fd","trace_flags":"01","resource.span.name":"_findReplicaSetUpdatesForUser","resource.span.links":[],"resource.span.attributed":{"code.filepath":"/usr/src/app/src/services/stateMachineManager/stateMonitoring/findReplicaSetUpdates.jobProcessor.ts","code.function":"_findReplicaSetUpdatesForUser","spid":0,"endpoint":"http://cn3_creator-node_1:4002"},"resource.service.name":"content-node","resource.service.spid":0,"resource.service.endpoint":"http://cn3_creator-node_1:4002","logLevel":"warn"}

Sync job failing for a wallet

{"name":"audius_creator_node","clusterWorker":"Worker 2","hostname":"51f7210dfa94","pid":231,"queue":"find-replica-set-updates-queue","jobId":"117","level":50,"msg":"_findReplicaSetUpdatesForUser(): Secondary http://cn4_creator-node_1:4003 for user 0xbc13d58475359394a75795e292bbe4aeb254cf18 encountered too many sync errors. Marking replica as unhealthy.","time":"2022-11-29T17:27:22.657Z","v":0,"trace_id":"d9bcac02dd1b8b5e197be43b74816f61","span_id":"1eb2bd430f18de60","trace_flags":"01","resource.span.name":"_findReplicaSetUpdatesForUser","resource.span.links":[],"resource.span.attributed":{"code.filepath":"/usr/src/app/src/services/stateMachineManager/stateMonitoring/findReplicaSetUpdates.jobProcessor.ts","code.function":"_findReplicaSetUpdatesForUser","spid":0,"endpoint":"http://cn3_creator-node_1:4002"},"resource.service.name":"content-node","resource.service.spid":0,"resource.service.endpoint":"http://cn3_creator-node_1:4002","logLevel":"error"}

Redis highlighting that a wallet-secondary-date recorded the max allowed fails:

127.0.0.1:6379> keys PRIMARY_TO_SECONDARY_SYNC_FAILURE*
1) "PRIMARY_TO_SECONDARY_SYNC_FAILURE::2022-11-29::http://cn2_creator-node_1:4001::0xbc13d58475359394a75795e292bbe4aeb254cf18"
2) "PRIMARY_TO_SECONDARY_SYNC_FAILURE::2022-11-29::http://cn4_creator-node_1:4003::0xbc13d58475359394a75795e292bbe4aeb254cf18"
127.0.0.1:6379> hgetall PRIMARY_TO_SECONDARY_SYNC_FAILURE::2022-11-29::http://cn2_creator-node_1:4001::0xbc13d58475359394a75795e292bbe4aeb254cf18
1) "failure_fetching_user_replica_set"
2) "10"
127.0.0.1:6379> PRIMARY_TO_SECONDARY_SYNC_FAILURE::2022-11-29::http://cn4_creator-node_1:4003::0xbc13d58475359394a75795e292bbe4aeb254cf18
127.0.0.1:6379> hgetall PRIMARY_TO_SECONDARY_SYNC_FAILURE::2022-11-29::http://cn4_creator-node_1:4003::0xbc13d58475359394a75795e292bbe4aeb254cf18
1) "failure_fetching_user_replica_set"
2) "10"

Monitoring - How will this change be monitored? Are there sufficient logs / alerts?

Check log surrounding "SecondarySyncHealthTracker":"computeWalletOnSecondaryExceedsMaxErrorsAllowed","msg":"Wallets on secondaries have exceeded the allowed error capacity for today for the wallets that exceed the max errors allowed
and also Wallet encountered max errors allowed on secondary for if max errors are encountered

…ing-perf-3

...r-node/src/services/stateMachineManager/stateReconciliation/issueSyncRequest.jobProcessor.ts

creator-node/test/secondarySyncHealthTracker.test.js

vicky-g · 2022-11-15T18:24:05Z

ignore tests for now. will update

theoilie

all looking good besides having the sync tracker as a class. that's my only big request to change here so most comments are about that or just nits

...-node/src/services/stateMachineManager/stateMonitoring/findReplicaSetUpdates.jobProcessor.ts

creator-node/src/services/stateMachineManager/stateReconciliation/SecondarySyncHealthTracker.ts

creator-node/src/services/stateMachineManager/stateMonitoring/monitorState.jobProcessor.ts

…fail

creator-node/test/asyncRetry.test.js

dev-tools/audius-compose

theoilie

great work - it was especially nice and reassuring to see the manual tests!

creator-node/src/services/stateMachineManager/stateReconciliation/SecondarySyncHealthTracker.ts

theoilie · 2022-11-29T18:15:54Z

.../src/services/stateMachineManager/stateReconciliation/SecondarySyncHealthTrackerConstants.ts

+  failure_fetching_user_replica_set: 10,
+  failure_force_resync_check: 10,
+  failure_fetching_user_gateway: 10,
+  failure_delete_db_data: 3,
+  failure_delete_disk_data: 3,
+  failure_sync_secondary_from_primary: 10,
+  failure_db_transaction: 3,
+  failure_export_wallet: 10,
+  failure_import_not_consistent: 3,
+  failure_import_not_contiguous: 3,
+  failure_inconsistent_clock: 10,
+  failure_undefined_sync_status: 3,
+  default: 5


flagging that we'll probably want to fine tune these and add other errors based on what we see in prod after release

technically these errors only get added from the recordFailure call. these fields are from the prometheus constants so if more errors come up, there needs to be a manual update here

agree these all seem reasonable for now and we can tweak later

creator-node/src/services/stateMachineManager/stateReconciliation/SecondarySyncHealthTracker.ts

creator-node/test/stateMonitoringUtils.test.js

SidSethi · 2022-11-30T19:18:36Z

.../src/services/stateMachineManager/stateReconciliation/SecondarySyncHealthTrackerConstants.ts

@@ -0,0 +1,19 @@
+// See prometheus.constants.js for reference on potential errors


maybe valuable to denote either in comment or in variable name that these are NUMBER_OF_DAILY_RETRIES

* add flag to disable sync tracking * disable recording sync * update env var * update test * remove extra }) * remove .only * WIP remove recordSuccess * update tests to remove recordSuccess * fix test * WIP * also wip * wip also redis * wip change names + redis calls * WIP * remove unused file * remove personal file * remove unused redis command * fix old comments + update retry + refactor * pass in serialized data but let class instance handle processing * add question mark + remove sync type * fix configs * remove additional properties * add sync undefined status response * remove comment * update some tests * fix some tests * fix test again * fix tests again * fix test * remove. only * fix tests * fix tests * fix some tests * fix tests again * remove .only * fix test? * write tests * fix test * cleanup * remove unused vars * move fn * short circuit * remove t/f map * fix tests * fix tests * remove forced failure

vicky-g added 16 commits October 31, 2022 21:35

add flag to disable sync tracking

0609759

disable recording sync

48475e9

update env var

d50f714

update test

2936504

remove extra })

e7f45ad

remove .only

bc987e8

Merge remote-tracking branch 'origin/main' into vg-improve-sync-track…

f87c031

…ing-perf-3

WIP remove recordSuccess

19868ad

update tests to remove recordSuccess

64f8308

fix test

eaf9d1a

Merge remote-tracking branch 'origin/main' into vg-improve-sync-track…

bb4411e

…ing-perf-3

WIP

0c28833

also wip

af3ccce

wip also redis

ae295d4

wip change names + redis calls

0465ba9

WIP

893dc6b

pull-request-size bot added the size/XXL label Nov 15, 2022

remove unused file

184c569

vicky-g commented Nov 15, 2022

View reviewed changes

...r-node/src/services/stateMachineManager/stateReconciliation/issueSyncRequest.jobProcessor.ts Outdated Show resolved Hide resolved

vicky-g commented Nov 15, 2022

View reviewed changes

creator-node/test/secondarySyncHealthTracker.test.js Outdated Show resolved Hide resolved

remove personal file

e577390

vicky-g requested a review from theoilie November 15, 2022 18:24

vicky-g added 2 commits November 16, 2022 17:55

remove unused redis command

8e96e99

fix old comments + update retry + refactor

b0e01c4

theoilie reviewed Nov 16, 2022

View reviewed changes

vicky-g added 4 commits November 18, 2022 16:59

pass in serialized data but let class instance handle processing

32673d7

add question mark + remove sync type

5421ed4

fix configs

b07508f

remove additional properties

19d7613

vicky-g added 9 commits November 28, 2022 19:17

fix some tests

8ba9694

fix tests again

ffeb098

remove .only

b519ed9

fix test?

8460c20

write tests

da31ec5

fix test

a67e42a

cleanup

cc7c902

Merge remote-tracking branch 'origin/main' into vg-ssht-change-record…

d96ddfa

…fail

remove unused vars

231bedf

vicky-g commented Nov 29, 2022

View reviewed changes

creator-node/test/asyncRetry.test.js Show resolved Hide resolved

move fn

956d86a

vicky-g requested a review from theoilie November 29, 2022 17:57

vicky-g marked this pull request as ready for review November 29, 2022 17:59

vicky-g commented Nov 29, 2022

View reviewed changes

dev-tools/audius-compose Show resolved Hide resolved

theoilie approved these changes Nov 29, 2022

View reviewed changes

short circuit

45a8c0d

vicky-g added the content-node Content Node (previously known as Creator Node) label Nov 29, 2022

vicky-g added 4 commits November 29, 2022 21:05

remove t/f map

93d7139

fix tests

4ded38a

fix tests

842df69

remove forced failure

19fe234

vicky-g commented Nov 30, 2022

View reviewed changes

creator-node/test/stateMonitoringUtils.test.js Show resolved Hide resolved

vicky-g requested a review from theoilie November 30, 2022 01:48

theoilie approved these changes Nov 30, 2022

View reviewed changes

vicky-g assigned SidSethi Nov 30, 2022

SidSethi reviewed Nov 30, 2022

View reviewed changes

vicky-g merged commit b292d3c into main Nov 30, 2022

vicky-g deleted the vg-ssht-change-recordfail branch November 30, 2022 19:43

vicky-g mentioned this pull request Dec 1, 2022

Remove unused sync env vars #4413

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SecondarySyncHealthTracker change recordfail #4353

SecondarySyncHealthTracker change recordfail #4353

vicky-g commented Nov 15, 2022 •

edited

vicky-g commented Nov 15, 2022

theoilie left a comment

theoilie left a comment

theoilie Nov 29, 2022

vicky-g Nov 29, 2022

SidSethi Nov 30, 2022

SidSethi Nov 30, 2022

		@@ -0,0 +1,19 @@
		// See prometheus.constants.js for reference on potential errors

SecondarySyncHealthTracker change recordfail #4353

SecondarySyncHealthTracker change recordfail #4353

Conversation

vicky-g commented Nov 15, 2022 • edited

Description

Tests

Monitoring - How will this change be monitored? Are there sufficient logs / alerts?

vicky-g commented Nov 15, 2022

theoilie left a comment

Choose a reason for hiding this comment

theoilie left a comment

Choose a reason for hiding this comment

theoilie Nov 29, 2022

Choose a reason for hiding this comment

vicky-g Nov 29, 2022

Choose a reason for hiding this comment

SidSethi Nov 30, 2022

Choose a reason for hiding this comment

SidSethi Nov 30, 2022

Choose a reason for hiding this comment

vicky-g commented Nov 15, 2022 •

edited