-
Notifications
You must be signed in to change notification settings - Fork 20
fix: Prevent blocking of the whole manager while cloning a vfolder #539
fix: Prevent blocking of the whole manager while cloning a vfolder #539
Conversation
Codecov Report
@@ Coverage Diff @@
## main #539 +/- ##
=======================================
Coverage 48.87% 48.87%
=======================================
Files 54 54
Lines 9025 9025
=======================================
Hits 4411 4411
Misses 4614 4614 Continue to review full report at Codecov.
|
user_uuid = str(user_uuid) | ||
group_uuid = None | ||
ownership_type = 'user' | ||
insert_values = { | ||
'id': folder_id.hex, | ||
'id': uuid.uuid4().hex, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is the original folder ID?
What happens if the folder ID is lost?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the good point! I thought that UUID and folder_id would always be the same. But, it was actually different and found that other tasks of vfolder had errors. so, I will fix it.
@@ -25,6 +25,7 @@ | |||
from aiohttp import web | |||
import aiohttp_cors | |||
import sqlalchemy as sa | |||
from ..background import ProgressReporter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reorder the import following other imports.
(What is the rule?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I reorder it in alphabetical order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try out finding by yourself. How are the current imports organized?
changes/539.fix
Outdated
@@ -0,0 +1 @@ | |||
Split the destination vfolder creation and actual clone operationsrun the clone operation as a background task and just return the bgtask ID in the vfolder clone API, so that clients could keep track of the progress and result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix typos and try to write it shorter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got it!
The PR itself also needs to split the txn block. Currently the storage proxy RPC call has race condition with the target vfolder creation. |
…e RPC calls * TODO: update storage-proxy to allow creation of destination vfolder with exist_ok=True option. (for backward compatibility)
) * Change the vfolder clone operation to run as a background task * Add `bgtask_id` in the response of vfolder clone API Co-authored-by: Joongi Kim <[email protected]> Backported-From: main (22.03) Backported-To: 21.09
) * Change the vfolder clone operation to run as a background task * Add `bgtask_id` in the response of vfolder clone API Co-authored-by: Joongi Kim <[email protected]> Backported-From: main (22.03) Backported-To: 21.03
resolved : lablup/backend.ai#354
prevent blocking of the whole manager while cloning a vfolder
Run the clone operation as a background task and just return the bgtask ID in the vfolder clone API, so that clients could keep track of the progress and result.
Split the destination vfolder creation and actual clone operations, and insert the database transaction to create the destination vfolder record between them.