-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Description
Expected Behavior
Whenever transactions are used with a Postgres online store, changes should be committed. If transactions are not required, we shouldn't open transactions (e.g. when simply reading features).
Current Behavior
We are using psycopg3 in case we use Postgres for the online store. If you create connections with psycopg3 using a context manager (i.e. a with ... block), it will automatically commit the transaction once you exit the context. However, in feast we are manually creating (and in the case of connection pools, putting) connections, so we need to ensure that we also explicitly call commit after firing queries to the database.
The commit statements were missing for (async) read methods of the PostgresOnlineStore and the teardown method. For the read statements, this wasn't an issue as we just do a SELECT. However, it does add warning statements and quite a bit of latency as transactions needed to be rolled back.
The warning statement:
2025-02-07 14:05:40,301 WARNING [psycopg.pool] rolling back returned connection: <psycopg.AsyncConnection [INTRANS] (host=<REDACTED> port=<REDACTED> user=<REDACTED> database=<REDACTED>) at 0x314498790>
Specifications
- Version: 0.40.1
- Platform: MacOS
- Subsystem: Sequoia 15.2
Possible Solution
Add missing commit calls and enable autocommit if a transaction is not needed for improved performance (e.g. for read methods).
Filed a PR here: #5039