Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

Further apply read-only txn hints #529

Merged
merged 5 commits into from
Feb 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions changes/529.fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Apply "read-only" attribute to a broader range of database transactions to improve overall performance
9 changes: 5 additions & 4 deletions src/ai/backend/manager/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from aiohttp import web
import aiohttp_cors
from aioredis import Redis
from aioredis.client import Pipeline as RedisPipeline
from dateutil.tz import tzutc
from dateutil.parser import parse as dtparse
import sqlalchemy as sa
Expand Down Expand Up @@ -437,12 +438,12 @@ async def _query_cred():
if row is None:
raise AuthorizationFailed('Access key not found')

async def _pipe_builder(r: Redis):
async def _pipe_builder(r: Redis) -> RedisPipeline:
pipe = r.pipeline()
num_queries_key = f'kp:{access_key}:num_queries'
pipe.incr(num_queries_key)
pipe.expire(num_queries_key, 86400 * 30) # retention: 1 month
await pipe.execute()
return pipe
Comment on lines -445 to +446
Copy link
Member

Choose a reason for hiding this comment

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

I am just curious.
is this a big change or difference?

Copy link
Member Author

@achimnol achimnol Feb 8, 2022

Choose a reason for hiding this comment

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

common.redis.execute() function accepts either a lambda to return an awaitable for Redis operation or a Redis pipeline object.
This change is to make it clearer to pass the pipeline object instead of an awaitable, though the final result is the same: executing the pipeline vs. executing an awaitable that executes the pipeline. Just a matter of style, but it reduces one extra function call.


await redis.execute(root_ctx.redis_stat, _pipe_builder)
else:
Expand Down Expand Up @@ -482,12 +483,12 @@ async def _query_cred():
if not secrets.compare_digest(my_signature, signature):
raise AuthorizationFailed('Signature mismatch')

async def _pipe_builder(r: Redis):
async def _pipe_builder(r: Redis) -> RedisPipeline:
pipe = r.pipeline()
num_queries_key = f'kp:{access_key}:num_queries'
pipe.incr(num_queries_key)
pipe.expire(num_queries_key, 86400 * 30) # retention: 1 month
await pipe.execute()
return pipe

await redis.execute(root_ctx.redis_stat, _pipe_builder)
else:
Expand Down
2 changes: 1 addition & 1 deletion src/ai/backend/manager/api/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ async def push_session_events(
if group_name == '*':
group_id = '*'
else:
async with root_ctx.db.begin() as conn:
async with root_ctx.db.begin_readonly() as conn:
query = (
sa.select([groups.c.id])
.select_from(groups)
Expand Down
2 changes: 1 addition & 1 deletion src/ai/backend/manager/api/resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ async def get_time_binned_monthly_stats(request: web.Request, user_uuid=None):
now = datetime.now(tzutc())
start_date = now - timedelta(days=30)
root_ctx: RootContext = request.app['_root.context']
async with root_ctx.db.begin() as conn:
async with root_ctx.db.begin_readonly() as conn:
query = (
sa.select([kernels])
.select_from(kernels)
Expand Down
Loading