Skip to content

Conversation

ericphanson
Copy link
Contributor

closes #232

Based on #233 which is why the diff is so huge here. That one should be merged first

@KristofferC
Copy link
Collaborator

The LAST_MUTATION_TIMESTAMP should per per API token or something like that, or? It feels somehow the wrong abstraction level to make this a single global of the whole package.

@ericphanson
Copy link
Contributor Author

True, it should be at the same granularity at which the rate limits are applied, which is per account iiuc. Per token seems like a reasonable approximation. I’m not sure a good way to do that, it would feel weird / insecure to make a global dictionary with tokens as keys. We could hash them or something. Or use the auth object itself (add a mutable field), but then if you created 2 auth objects with the same token they wouldn’t share timestamps.

@ericphanson
Copy link
Contributor Author

ChatGPT suggested the key to be

reinterpret(UInt64, sha256(token)[1:8])

and cleaning up old entries to not accumulate memory forever in a long running process with many tokens used.

@ericphanson
Copy link
Contributor Author

ok, it adds a fair bit of complexity to track the state across auth instances but I think the current state does so:

  • NEXT_AVAILABLE::Dict{UInt64, Float64} maps token-hashes to availabilities (i.e. next time that auth is allowed to make a mutation)
  • cleans up old key-value pairs periodically to prevent memory leaks
  • aims to be not-terrible in concurrent scenarios: global data structures protected by locks, uses reservations instead of last-used-timestamps so tasks wait the correct amount of time instead of looping 1s waits until ready

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

implement pause between mutations
2 participants