[CON-234] Enable write quorum

[theoilie]

Description

Libs / SDK

Add writeQuorumEnabled flag to libs creatorNodeConfig passed in via libs/SDK constructor. This flag is used to pass the Enforce-Write-Quorum header in every call from the libs/SDK CreatorNode API. The routes on Content Node can then decide how they want to consume the header (including just ignoring it when not applicable).

Content Node

• Change enforceWriteQuorum env var default from false to true
• Make file consumers of the issueAndWaitForSecondarySyncRequests not await it because discovery only indexes metadata, so write quorum isn't needed on files
  • This applies to the routes: /image_upload, /audius_users, /playlists, /tracks
• Make metadata consumers of the issueAndWaitForSecondarySyncRequests middleware await it so that the middleware can actually block the request from succeeding until a 2/3 write quorum is reached
  • This makes writes await replication in all scenarios, but the write can still succeed if replication fails and write quorum is disabled
  • This applies to the routes: /audius_users/metadata, /playlists/metadata, /tracks/metadata
• Change order of precedence for enforcing client header vs server env var:
  • If client passes Enforce-Write-Quorum header as false, disable write quorum regardless of the enforceWriteQuorum env var
  • If client passes Enforce-Write-Quorum header as true, enable write quorum regardless of the enforceWriteQuorum env var
  • If client doesn't pass Enforce-Write-Quorum header, write quorum = enforceWriteQuorum env var
• Add audius_cn_write_quorum_duration_seconds histogram to track write quorum success/failure

Histogram data after changing profile picture and uploading a track once with write quorum enabled and then changing profile picture and uploading a track once with write quorum disabled:

Tests

Env Var + Header Order of Precedence

I tested all the env var (true/false) + header (true/'true'/false/'false'/null/'null'/undefined) + ignoreWriteQuorum (true/false) combos to make sure the final enforceWriteQuorum variable used in the middleware is correct.

Manual E2E Test Setup

Before each test:

1. (remote box) Disable snapback (set disableSnapback env var to true)
2. (remote box) Disable refactored state monitoring (set stateMonitoringQueueRateLimitJobsPerInterval env var to 0) so that we can test only seeing manual syncs and not recurring
3. (remote box) Check out this branch of the protocol and restart: git checkout theo-write-quorum && A down && A up
4. (local) Check out this branch of the protocol and run libs so the client can consume its changes: git checkout theo-write-quorum && cd libs && npm link && npm run dev
5. (local) Check out latest from client, link local libs, and start it:
   • cd audius-client/packages/web
   • npm link @audius/sdk
   • scp -r <REMOTE_BOX>:.audius ~/
   • npm run start:dev:cloud

Manual E2E Tests

Set local storage to make the feature flag enable/disable write quorum by entering the following into the Console:
localStorage.setItem('FeatureFlagOverride:write_quorum_enabled', 'enabled');
OR
localStorage.setItem('FeatureFlagOverride:write_quorum_enabled', 'disabled');

1. Write quorum disabled + default processSync behavior:
   • POST request to /audius_users/metadata when uploading track enqueued+awaited manual sync without enforcing failing the POST request if the sync failed
   • Successfully logged completion before returning 200: issueAndWaitForSecondarySyncRequests - At least one secondary successfully replicated content for user 0xd7903b72f7e92f68ab14e61ef65689a201c4d741 in 534ms","time":"2022-07-08T00:20:17.617Z" "requestMethod":"POST","requestHostname":"cn4_creator-node_1","requestUrl":"/audius_users/metadata","requestWallet":"0xd7903b72f7e92f68ab14e61ef65689a201c4d741","requestBlockchainUserId":"2","duration":627,"statusCode":200
2. Write quorum enabled + processSync temporarily changed to always throw:
   • Progress bar looks super glitchy – it bounced back and forth - and then eventually turns into heavy load error page
   • Primary eventually returned 500 for the metadata POST request with the expected error: Error processing request: Error: issueAndWaitForSecondarySyncRequests Error - Failed to reach 2/3 write quorum for user
3. Write quorum enabled + manualSyncsDisabled=true:
   • Throws error issueAndWaitForSecondarySyncRequests Error - Cannot proceed due to manualSyncsDisabled
   • There haven't been any occurrences of this in Loggly for the past 7 days
4. Write quorum enabled + missing req.session.wallet:
   • Throws error issueAndWaitForSecondarySyncRequests Error - req.session.wallet missing
   • There haven't been any occurrences of this in Loggly for the past 7 days
5. Write quorum enabled + node is not primary or invalid creatorNodeEndpoints:
   • Throws error issueAndWaitForSecondarySyncRequests Error - Cannot process sync op - this node is not primary or invalid creatorNodeEndpoints
   • There haven't been any occurrences of this in Loggly for the past 7 days

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

• Search for any errors starting with issueAndWaitForSecondarySyncRequests Error and read the test scenarios and issueAndWaitForSecondarySyncRequests code for more context
• Keep an eye on the new audius_cn_write_quorum_duration_seconds metric -- look for the result label equaling anything other than succeeded

[theoilie]

just a draft to make sure this is on the right track. I bolded a couple TODO questions in the PR description if someone has an answer to those

[dmanjunath]

first off thank you so much for testing this out and the great writeup. just had a few small suggestions but overall changes look good so far and i'm glad we're finally trying to get this out.

[SidSethi]

thank you for putting in so much thought here and the thorough PR description 🙌

[theoilie]

note that there's a bug in the axios version of libs that changes headers from null to [object Object]. this appears to be fixed in v0.20.0, but I'd rather not risk updating since I've read other users having issues after upgrading. the workaround I'm using is to pass the null as a string

see axios solution PR and problem issue 1 and issue 2

[theoilie]

@SidSethi or @dmanjunath ready for review! I did some more testing, updated the PR description, and added a metric so we can track the rollout on staging and then prod. since there are only 7 routes that use write quorum, I think it's worth the slightly higher cardinality to have the route label that I added. If you're strongly opposed to this, though, I can change it to just be an ignoreWriteQuorum label. note that the ignoreWriteQuorum and enforceWriteQuorum labels should be the same for the same route on each request, so that lack of permutation should keep the cardinality down to a reasonable number

also I'll look into why the tests are failing tomorrow. I suspect it's because I changed the other routes to await write quorum

[dmanjunath]

This looks great to me, thanks for making these changes. I'm actually fine with the cardinality, don't see a problem with that at all, I love the observability 😃 . I'll let @SidSethi give the final approve but nothing else on my end

[SidSethi]

nice!! excited to get this out - i appreciate the very detailed PR description and testing, and all the thought you put in to reason through all the underlying complexity here

also interesting point about the ignoreWriteQuorum thing - your point that metadata must be indexed but the associate routes don't need to be is a good one. my initial logic was that the associate routes are what actually cause a chain write which leads to indexing, but it makes sense that the chain write just writes a metadata CID which should already have been replicated by the first route. tricky to reason through, but i think it makes sense!

btw - i think our standard is to have separate PRs for each service e.g. libs and CN here (i know this defeats the point of a monorepo lol)
there'll also need to be a separate libs PR with new version (yeah i know)
idk if @dmanjunath cares

[dmanjunath]

@theoilie @SidSethi i think we can have libs code merged as part of this PR. we should just bump and merge the libs version separately

[theoilie]

sounds good, just waiting to confirm that tests now pass on CI and then I'll merge!