perf(I/O): rm blocking fs calls

[kishore03109]

Problem

We have quite a number of blocking I/O calls that has lead to unwanted CPU spikes in the event pool thread.

This PR scrapes any existence of I/O calls and ensures that there is no lingering sync fs calls made from node.

Solution

use fs.promises instead

Looking at the spike after running the site checker

NOTE: initial spike in event loop delay is due to the startup push to staging environment, this is not due to the site checker calls that were made

Breaking Changes

• Yes - this PR contains breaking changes
  • Details ...
• No - this PR is backwards compatible with ALL of the following feature flags in this doc

Features:

when running the site repair form:

• Details ...

Improvements:

• Details ...

Bug Fixes:

• Details ...

Before & After Screenshots

BEFORE:

AFTER:

Tests

• Run the following command from the command line:

grep -rE '(accessSync|appendFileSync|chmodSync|chownSync|closeSync|copyFileSync|cpSync|existsSync|fchmodSync|fchownSync|fdatasyncSync|fstatSync|fsyncSync|ftruncateSync|futimesSync|lchmodSync|lchownSync|lutimesSync|linkSync|lstatSync|mkdirSync|mkdtempSync|opendirSync|openSync|readdirSync|readFileSync|readlinkSync|readSync|readvSync|realpathSync|renameSync|rmdirSync|rmSync|statSync|statfsSync|symlinkSync|truncateSync|unlinkSync|utimesSync|writeFileSync|writeSync|writevSync)\b' src 

the only results should be from the GitFileSystemService.spec.ts which is fine since this test file runs locally and not in prod line

• Submit a form here for a repo in staging efs, and assert that the attachments are sent properly

• submit the site create form

Deploy Notes

New environment variables:

• env var : env var details
  • added env var to 1PW + SSM script (fetch_ssm_parameters.sh)

New scripts:

• script : script details

New dependencies:

• dependency : dependency details

New dev dependencies:

• dependency : dependency details

[kishore03109]

• fix(link checker): wrong error reported #1227
• perf(I/O): rm blocking fs calls #1220 👈
• develop

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @kishore03109 and the rest of your teammates on Graphite

[seaerchin]

can we make sure that users see a sensible error message if we do a throw here?

[seaerchin]

we don't have to fix this now but note that we can't differentiate between SiteCheckerError and (an imaginary) FolderLockError

[seaerchin]

double checking - we're checking if we can access the newly created lock file right? in the event that we can't, should we also perform some clean-up? (we could file a ticket for this, no worri3es)

[kishore03109]

nope, this.createLogsFolder is just a sanity check that the top level folder exists.
the key thing that this function does is check if the checker.lock exists, which is done by the fs.promise.access call.

the lock file creation is done in a separate function call. the clean up functionality already exists (if some error exists, create a error file + remove the checker.lock file)

[seaerchin]

hmm what i'm hearing is that the point of isCurrentlyLocked is the check on checker.lock! seems abit weird to have the log creation folder here then? we can probably shift it out and this would be abit less confusing. (since this PR is scoped to fixing fsSync, we can do it later)

[kishore03109]

the dir structure is

• pa-corp (folder)
  • logs.csv (file)
  • checker.lock (file)

what we are doing here is ensuring that the top level folder exists before checking for the checker.lock ya

[seaerchin]

lgtm, checked using \.[a-zA-Z]+sync and only statSync in tests + neverthrow's async methods could be found.

[seaerchin]

@kishore03109 as a follow-up, the next time we submit a form that triggers an infra flow on our backend, could we verify that CPU usage doesn't spike anymore?

[kishore03109]

@kishore03109 as a follow-up, the next time we submit a form that triggers an infra flow on our backend, could we verify that CPU usage doesn't spike anymore?

@seaerchin updated the pr desc for this