possible race condition: (sqlite3.IntegrityError) UNIQUE constraint failed: tokens.repo

[TomasTomecek]

DEBUG:urllib3.connectionpool:Starting new HTTPS connection (1): api.github.com:443
DEBUG:urllib3.connectionpool:https://api.github.com:443 "POST /app/installations/11261676/access_tokens HTTP/1.1" 201 229
ERROR:tokman.app:Exception on /api/admdev8/tickgit [GET]
Traceback (most recent call last):
File "/usr/lib64/python3.11/site-packages/sqlalchemy/engine/base.py", line 1900, in _execute_context
    self.dialect.do_execute(
File "/usr/lib64/python3.11/site-packages/sqlalchemy/engine/default.py", line 736, in do_execute
    cursor.execute(statement, parameters)

sqlite3.IntegrityError: UNIQUE constraint failed: tokens.repo
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/usr/lib/python3.11/site-packages/flask/app.py", line 1820, in full_dispatch_request
    rv = self.dispatch_request()
File "/usr/lib/python3.11/site-packages/flask/app.py", line 1796, in dispatch_request
    return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)
File "/usr/lib/python3.11/site-packages/flask_restx/api.py", line 405, in wrapper
    resp = resource(*args, **kwargs)
File "/usr/lib/python3.11/site-packages/flask/views.py", line 107, in view
    return current_app.ensure_sync(self.dispatch_request)(**kwargs)
File "/usr/lib/python3.11/site-packages/flask_restx/resource.py", line 46, in dispatch_request
    resp = meth(*args, **kwargs)
File "/tokman/tokman/app.py", line 118, in get
    db.session.commit()
File "<string>", line 2, in commit
File "/usr/lib64/python3.11/site-packages/sqlalchemy/orm/session.py", line 1451, in commit
    self._transaction.commit(_to_root=self.future)
File "/usr/lib64/python3.11/site-packages/sqlalchemy/orm/session.py", line 829, in commit
    self._prepare_impl()
File "/usr/lib64/python3.11/site-packages/sqlalchemy/orm/session.py", line 808, in _prepare_impl
    self.session.flush()
File "/usr/lib64/python3.11/site-packages/sqlalchemy/orm/session.py", line 3444, in flush
    self._flush(objects)
File "/usr/lib64/python3.11/site-packages/sqlalchemy/orm/session.py", line 3583, in _flush
    with util.safe_reraise():
File "/usr/lib64/python3.11/site-packages/sqlalchemy/util/langhelpers.py", line 70, in __exit__
    compat.raise_(
File "/usr/lib64/python3.11/site-packages/sqlalchemy/util/compat.py", line 211, in raise_
    raise exception
File "/usr/lib64/python3.11/site-packages/sqlalchemy/orm/session.py", line 3544, in _flush
    flush_context.execute()
File "/usr/lib64/python3.11/site-packages/sqlalchemy/orm/unitofwork.py", line 456, in execute
    rec.execute(self)
File "/usr/lib64/python3.11/site-packages/sqlalchemy/orm/unitofwork.py", line 630, in execute
    util.preloaded.orm_persistence.save_obj(
File "/usr/lib64/python3.11/site-packages/sqlalchemy/orm/persistence.py", line 245, in save_obj
    _emit_insert_statements(
File "/usr/lib64/python3.11/site-packages/sqlalchemy/orm/persistence.py", line 1238, in _emit_insert_statements
    result = connection._execute_20(
File "/usr/lib64/python3.11/site-packages/sqlalchemy/engine/base.py", line 1705, in _execute_20
    return meth(self, args_10style, kwargs_10style, execution_options)
File "/usr/lib64/python3.11/site-packages/sqlalchemy/sql/elements.py", line 334, in _execute_on_connection
    return connection._execute_clauseelement(
File "/usr/lib64/python3.11/site-packages/sqlalchemy/engine/base.py", line 1572, in _execute_clauseelement
    ret = self._execute_context(
File "/usr/lib64/python3.11/site-packages/sqlalchemy/engine/base.py", line 1943, in _execute_context
    self._handle_dbapi_exception(
File "/usr/lib64/python3.11/site-packages/sqlalchemy/engine/base.py", line 2124, in _handle_dbapi_exception
    util.raise_(
File "/usr/lib64/python3.11/site-packages/sqlalchemy/util/compat.py", line 211, in raise_
    raise exception
File "/usr/lib64/python3.11/site-packages/sqlalchemy/engine/base.py", line 1900, in _execute_context
    self.dialect.do_execute(
File "/usr/lib64/python3.11/site-packages/sqlalchemy/engine/default.py", line 736, in do_execute
    cursor.execute(statement, parameters)

sqlalchemy.exc.IntegrityError: (sqlite3.IntegrityError) UNIQUE constraint failed: tokens.repo
[SQL: INSERT INTO tokens (repo, token, expires_at) VALUES (?, ?, ?)]
[parameters: ('admdev8/tickgit', 'ghs_XXXXXXXXX', '2023-02-25 01:46:20.000000')]

https://red-hat-it.sentry.io/issues/3556205037/?query=is%3Aunresolved&referrer=issue-stream
https://red-hat-it.sentry.io/issues/3952931168/?query=is%3Aunresolved&referrer=issue-stream

[csomh]

🤦

Thanks! Nice catch! This means it can and will happen... I guess I'll need to reconfigure tokman to use a single worker. Any better idea?

[TomasTomecek]

Could we in theory utilize SQL transactions and make it work with 2 workers?

[csomh]

We already do that, I think... but even that wouldn't help. The error is triggered, b/c the repo name is expected to be unique in the DB so that we don't allow duplicate to tokens. The issue is that by the time this error occurred a new token was already requested, and the previous one is invalidated by this—at least this is how it worked in the past. I could check if GitHub changed behaviour since then.

[TomasTomecek]

Hm, interesting. Could we also make the repo name not unique and return the latest issued token?

[csomh]

We could, but then there would be very little benefit to run tokman, as workers could still end up trying to use invalid tokens, and ending up with failed GitHub requests.

[TomasTomecek]

Adding discuss label so we can come up with next steps here. Scaling down to a single worker sounds like the best short term solution.

[csomh]

It turns out that the GitHub API behaviour tokman was ment to work around is no longer a thing 😄 Tokens seem to stay valid, even after a new token was requested for the same installation.

So yeah, let's discuss what to do with this.

[TomasTomecek]

whaaaaaat, very nice!