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

Deletes are not respecting the time boundaries with CockroachDB #3753

Open
3 of 5 tasks
david7joy opened this issue Mar 29, 2024 · 1 comment
Open
3 of 5 tasks

Deletes are not respecting the time boundaries with CockroachDB #3753

david7joy opened this issue Mar 29, 2024 · 1 comment
Labels
bug Something is not working.

Comments

@david7joy
Copy link

Preflight checklist

Ory Network Project

No response

Describe the bug

We are observing that a record is being attempted to be deleted which was created as recently as 5 mins ago.
Hydra is pushing for a delete which may not be necessary or requested in cockroachDB.

Here is the delete query example :

DELETE FROM
  hydra_oauth2_access AS hydra_oauth2_access
WHERE
  nid = '39bba160-9c5d-4bd1-9f80-170c4f7c4a06':::UUID AND request_id = '591b4158-1431-47e7-b707-66b75b5edf75':::STRING

Here is the timestamp we observed in CockroachDB :

select crdb_internal.approximate_timestamp(crdb_internal_mvcc_timestamp) FROM hydra_oauth2_access AS hydra_oauth2_access WHERE nid = '39bba160-9c5d-4bd1-9f80-170c4f7c4a06' AND request_id ='591b4158-1431-47e7-b707-66b75b5edf75';
  
  crdb_internal.approximate_timestamp
---------------------------------------
  2024-03-28 18:46:16.506874

Observation: The transaction was only created 5 mins ago, but Hydra is attempting a delete.

It may be a good idea to use CRDB Built-in Row Level TTL capability over adding delete in Hydra.

Reproducing the bug

  • Use CockroachDB dedicated
  • Run > 55K tps

Relevant log output

No response

Relevant configuration

No response

Version

v2.2.0 and v2.2.0-rc.2

On which operating system are you observing this issue?

Linux

In which environment are you deploying?

Kubernetes

Additional Context

@viragtripathi @nollenr

@david7joy david7joy added the bug Something is not working. label Mar 29, 2024
@aeneasr
Copy link
Member

aeneasr commented Apr 5, 2024

Under normal circumstances, rows in hydra_oauth2_access are not deleted within 5 minutes, as the default expiry for access tokens is at least an hour. Depending on the clean up strategy (e.g. using Ory Hydra Janitor), one can choose how much time should pass before these stale records are removed.

Cockroach TTL is in our view not the best solution here as our SQL migrations are immutable files. Operators however want to choose how long they want to keep these records on file as it is often used in forensic investigations around account takeover (Answering: "who issued which token at what time and used it for what?"). Since we don't know how long these recods should be kept, we can't set a fixed time for TTL, which would be the case with row level TTL. I'm sure there is a way to engineer around this, but we believe that the Janitor is good enough even for larger-scale environments.

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

No branches or pull requests

2 participants