Backend >= 1.0.31 has high error rate and transaction times in CL prod environment

[mfugate1]

I attempted to deploy 1.0.33 to prod over the weekend while traffic was minimal, but as soon as it finished deploying, new relic reported incidents because of long transaction times and high error rates. I also tried deploying 1.0.35 and 1.0.31, same issue, so it looks like whatever is causing this was introduced in 1.0.31.

The transaction that was causing the issue is a POST to /api/mark. This operation typically executes in under 20ms, but during the incident it was at 52 seconds.

The error rate averaged about 30%, but I did not see any errors that aren't already occurring on 1.0.30, so I suspect that the issue lies with the POST I mentioned above.

But in case it happens to be relevant, the error during the incident was:
Unknown Error: QueryFailedError: null value in column "type" violates not-null constraint

Here is the breakdown from new relic for the /api/mark call. Looks like an issue in the code and not a database call

Here's what the new relic graphs looked like. Transaction time and error rate both spiked immediately when I deployed the new backend version

[jerith666]

Here's what changed between 1.0.30 and 1.0.31:
upgrade-backend-v1.0.30...upgrade-backend-v1.0.31

[SritterCL]

So where do we stand on this? @VivekFitkariwala - do you think the refactoring fixed this bug? Are you waiting for someone else to test this?

[mfugate1]

It is unlikely that Vivek's commit will fix this issue, but I will try deploying it to prod this weekend anyways just to make sure.

[SritterCL]

Can we reproduce this on QA first?

[mfugate1]

Until the locust script is updated, I'd have to set AUTH_CHECK=false for QA so it doesn't have to authenticate when calling experiment endpoints. If we're ok with having the API wide open I can make that change

[msandbothe]

I'm going to work on the locust script now to add an option that goes against existing experiments. It should be simple, so leave the AUTH_CHECK on in QA and we can hopefully test in QA tomorrow.

[VivekFitkariwala]

Added a PR for filtering experiments which are not assigned any condition.

[mfugate1]

Mentioned this on slack but realized I should add it here as well.

I merged that PR into main and deployed 1.0.39 into prod but still observed errors for api/mark calls

TypeError: Cannot destructure property 'experiment' of 'undefined' or 'null'.
    at ExperimentAssignmentService.<anonymous> (/usr/src/app/packages/Upgrade/dist/src/api/services/ExperimentAssignmentService.js:80:19)
    at Generator.next (<anonymous>)
    at fulfilled(/usr/src/app/packages/Upgrade/node_modules/tslib/tslib.js:112:62)
    at processTicksAndRejections (internal/process/task_queues.js:85:5)```

[nirmalpatel]

@mfugate1 can you see if this PR #57 fixes that issue? api/mark was producing errors when someone requested experiments for points and IDs that did not have any experiments associated with them. Vivek has fixed that in the mentioned PR. Maybe after some of those experiments got deleted we are now getting these errors...