Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collisions in updated_at when POSTing late_bytes #86

Open
kavorite opened this issue Jun 27, 2024 · 6 comments · May be fixed by #90
Open

Collisions in updated_at when POSTing late_bytes #86

kavorite opened this issue Jun 27, 2024 · 6 comments · May be fixed by #90
Labels
bug Something isn't working

Comments

@kavorite
Copy link
Contributor

kavorite commented Jun 27, 2024

This report comes from @Bash-09. The offending lines are these:

session_id, old_late_bytes = conn.execute(
sa.text(
"""SELECT session_id, late_bytes FROM demo_sessions
WHERE steam_id = :steam_id
AND updated_at = (
SELECT MAX(updated_at) FROM demo_sessions WHERE steam_id = :steam_id
);
""",
),
{"steam_id": steam_id},
).one()

Clients periodically get a 500 during header overwrites because this query returns multiple results, causing one() to throw an exception.

The easy fix is to use first() here instead of one(); still, I think it merits an investigation into why multiple entries of demo_sessions with identical updated_at fields exist. More comprehensive changes to the database schema could also make it easier to disambiguate which session the requester last interacted with.

@kavorite kavorite added the bug Something isn't working label Jun 27, 2024
@kavorite kavorite changed the title Identical updated_at? Collisions in updated_at when POSTing late_bytes Jun 27, 2024
kavorite added a commit that referenced this issue Jun 27, 2024
@jayceslesar
Copy link
Contributor

this is likely someone running tests against the system no? someone opening and closing sessions insanely fast?

@kavorite
Copy link
Contributor Author

certainly not. It was just bash running a mainline build. The start and end times checked out, so I think updated_at may have been overwritten retroactively.

@kavorite
Copy link
Contributor Author

For additional details see #87

@jayceslesar
Copy link
Contributor

@jayceslesar
Copy link
Contributor

stupid race conditions

@jayceslesar
Copy link
Contributor

jayceslesar commented Aug 17, 2024

@Bash-09 I can implement a non breaking optional param on the API side that can accept session_id as an optional param in the body of this request so that we can lazily implement that on the client backend if we think its worth it. This will allow us to explicitly update the row in the database that we need to instead of looking for what we should be updating. This is just a little scary territory because we can't control what version of the backend the clients are actually using so need to allow both and eventually deprecate and return a response that says "update your damn client" or something

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants