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.

Sign up for GitHub

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

Try to fix amount of connecitons #570

Merged
merged 4 commits into from
Dec 5, 2024
Merged

Try to fix amount of connecitons #570

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
79d3d34
Reduce pool size to 1
kongzii Dec 5, 2024
a90a189
only call create_all if necessary
kongzii Dec 5, 2024
280e5d6
edits based on rabbit
kongzii Dec 5, 2024
6b70e68
maybe fix
kongzii Dec 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion prediction_market_agent_tooling/tools/db/db_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def __init__(self, api_keys: APIKeys | None = None) -> None:
sqlalchemy_db_url.get_secret_value(),
json_serializer=json_serializer,
json_deserializer=json_deserializer,
pool_size=10,
pool_size=1,
pool_recycle=3600,
echo=True,
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Reconsider the drastic reduction in pool_size and debug logging

  1. Reducing pool_size to 1 eliminates concurrent database operations and may cause performance bottlenecks. Consider:

    • Using a size based on your concurrent operation needs (typically 5-10)
    • Implementing connection timeouts
    • Adding error handling for connection issues

  2. echo=True enables SQLAlchemy debug logging which may impact performance in production.

Consider this alternative configuration:

-    pool_size=1,
-    pool_recycle=3600,
-    echo=True,
+    pool_size=5,  # Adjust based on concurrent operation needs
+    pool_recycle=3600,
+    pool_timeout=30,  # Add timeout for connection acquisition
+    pool_pre_ping=True,  # Verify connections before using them
+    echo=False,  # Disable debug logging in production
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pool_size=1,
pool_recycle=3600,
echo=True,
)
pool_size=5, # Adjust based on concurrent operation needs
pool_recycle=3600,
pool_timeout=30, # Add timeout for connection acquisition
pool_pre_ping=True, # Verify connections before using them
echo=False, # Disable debug logging in production
)

Expand All @@ -52,6 +52,7 @@ def get_connection(self) -> Generator[Connection, None, None]:
def create_tables(
self, sqlmodel_tables: Sequence[type[SQLModel]] | None = None
) -> None:
# Determine tables to create
if sqlmodel_tables is not None:
tables_to_create = []
for sqlmodel_table in sqlmodel_tables:
Expand Down
Loading