bluesky-social / atproto

Social networking technology created by Bluesky
Other
6.49k stars 453 forks source link

Sporadic CRUD failure in CI #271

Closed dholms closed 1 year ago

dholms commented 2 years ago

CRUD tests are sporadically failing in CI.

Last I saw it was on racing deletes.

It doesn't happen all the time, doesn't happen on reruns and can't replicate locally.

dholms commented 2 years ago

Got it here: https://github.com/bluesky-social/atproto/actions/runs/3332711017/jobs/5513897190

dholms commented 2 years ago

wait i just got this for the first time locally. so this seems to be an actual issue & not CI

Still on racing deletes

devinivy commented 2 years ago

I poked at this a bit and found that the source of the error is hitting this condition in the mst's delete() method: https://github.com/bluesky-social/atproto/blob/70c525f80626b681caaaf80561e25cea5e62f960/packages/repo/src/mst/mst.ts#L330

dholms commented 2 years ago

hmmm seems like the most likely thing is that a delete misshaping the MST somehow and either making the key unfindable or deleting multiple keys

Sounds like this is probably my domain then.

Oh actually! A way to narrow this down more: serialize the requests to check if it's an issue with races or with the MST impl

pfrazee commented 2 years ago

$10 says serializing the requests affects timing and hides the bug again

On Mon, Nov 7, 2022 at 1:25 PM Daniel Holmgren @.***> wrote:

hmmm seems like the most likely thing is that a delete misshaping the MST somehow and either making the key unfindable deleting multiple keys

Sounds like this is probably my domain then.

Oh actually! A way to narrow this down more: serialize the requests to check if it's an issue with races or with the MST impl

— Reply to this email directly, view it on GitHub https://github.com/bluesky-social/atproto/issues/271#issuecomment-1306083530, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJWCU4QYO2I7RBICNU2OZTWHFJUBANCNFSM6AAAAAAROO3QY4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

dholms commented 2 years ago

yeah i have tests for serialized deletes on the MST & it passes fine 🤔