[EngCaioFonseca]

Pull Request Change Description

Closes #802
This PR bounds long-running CollectOSS queries and abandoned transactions after an 8Knot instance or worker disappears.

Celery’s soft time limit cannot reliably interrupt a blocking database-driver call, and PostgreSQL may not detect a disconnected client until a subsequent socket interaction. The changes add server-side limits and disconnect checks so cleanup does not depend solely on client-side shutdown handlers.

Changes

• Centralize source connections in CollectOSSManager. Worker queries use the manager’s SQLAlchemy engine, with connection pooling and context-managed connection return. The separate connection-options helper and duplicated source configuration were removed.
• Stream query results through server-side cursors. Results are fetched in bounded batches before being written to the PostgreSQL cache.
• Handle inherited pools after a fork. Forked workers call engine.dispose(close=False) before reusing an inherited engine, giving the child its own pool while preserving the parent’s connections.
• Configure PostgreSQL safeguards. Source connections set statement_timeout, idle_in_transaction_session_timeout, and server-side TCP keepalives.
• Enable disconnect polling where supported. On PostgreSQL 14+, the manager attempts to set client_connection_check_interval = '10s'. If the database host does not support it, a warning is logged and the configured timeouts still apply.
• Prevent repeated retries of canceled cache queries. The cache wrapper preserves QueryCanceled, including when SQLAlchemy wraps the driver exception, so Celery’s automatic retry mechanism excludes these failures.
• Make regression tests runnable through pytest. The database-backed tests live under tests/ and can be run with uv run pytest.

This addresses instance and worker disconnections. Browser-tab closure and navigation cancellation are tracked separately in #358.

Generative AI disclosure

Please select one option:

• This contribution was NOT assisted or created by Generative AI tools.
• This contribution was assisted or created by Generative AI tools.

If AI tools were used, please provide details below:
- What tools were used? Claude Opus 5, GPT 5.6 Sol
- How were these tools used? Investigation, initial draft, review.
- Did you review these outputs before submitting this PR? Yes, reviewed it.

[cdolfi]

Tested this out locally. I opened up 8knot and searched for a few repos, looked at the running queries on the db and then closed the tab. When I checked the queries after I closed it I still saw some of the queries still running from my now closed 8knot window

[MoralCode]

then closed the tab.

The scope of this PR by my understanding is canceling queries when the instance shuts down. Its very confusingly similar language to the original goal of "when a user closes the tab". Maybe its a phase 1 or something. I'm not sure if I fully remember the context or how this scope changed but the linked issue seems to have a title that reflects this "whole instance shutdown" scope too

[EngCaioFonseca]

Yes, @MoralCode is right. So the goal of the PR and the issue so far is to be able to eliminate the persistent queries even after app/instance shutdown. Which it's working on this PR. Closing the window/tab of 8Knot and being able to kill the queries could be a next step, although open for discussion as well, because, do we really want to kill the queries just after a tab closes? If a user closes it accidentally, everything would have to start again, in terms of queries. We could maybe have a time running after a tab is closed and then if not reopened, then all queries are eliminated. I think it's an option.
But for this PR, I'd say, having the time limits, and being able to validate that no query will remain open after the instance is shutdown, already solves in a significant way this issue with the queries.

[MoralCode]

validate that no query will remain open after the instance is shutdown, already solves in a significant way this issue with the queries.

I disagree. We are only shutting 8Knot down in prod once a week (and thats because we have a special process, i imagine other instances shut down less often than that). The main argument i see for this could be for development systems.

If queries are still active in postgres when 8knot disappears, that sounds like a potential postgres configuration issue

[cdolfi]

After testing current dev, queries are already shutting down when the app is being brought down. So we do not need change anything for that functionality. I agree with adrain as the issue of users closing tabs but the queries running for long periods was the problem that #802 was attempting to describe

[MoralCode]

I agree with adrain

That wasnt what i was saying. IMO #802 is a potentially valid (if lacking in detail/evidence) independent issue report that is clearly talking about instance shutdown. IMO we should put any evidence of queries staying open after instance shutdown there (or close it due to lack of reproducibility), and maybe shelve it until that issue becomes more pressing than whats in our current roadmap. I dont want to lose the research that has happened so far, but this is a fairly small performance fish.

IMO conversation about canceling no-longer-needed queries on tab closure or user navigation should go in #358

[EngCaioFonseca]

Yes, I agree. I'm going to keep this PR open dealing with persistent queries after app is shutdown, and do more investigation on the #358 to check if we can reproduce it or if that's still happening, then we, potentially (that's the goal), already have the solution to that in this PR.

For the issue we were discussing, being able to eliminate persistent queries after the tab/window is closed, I'm working on another branch in order to deal with that.

[cdolfi]

update augur names

[MoralCode]

Not seeing anything too egregiously wrong.

I do want to echo the feedback that going forward we should ensure new PRs are helping us move towards #1191, rather than introducing new variables prefixed with the name of the previous data collection db

[cdolfi]

@EngCaioFonseca lmk when comments have been addressed and I can test

[cdolfi]

tested and lgtm

[MoralCode]

Does this mean we are duplicating the connection pool for each worker? i.e. if the connection pool is 5 connections, this fork process means there are now 10 connections across both pools?