Skip to content

Commit 683eaf7

Browse files
committed
Fix: Swap file rename and DB project version commit to avoid orphaned versions in DB
If os.rename failed for moving uploaded data we would end up in broken project with project version in DB but no actual data.
1 parent 257e16d commit 683eaf7

2 files changed

Lines changed: 35 additions & 9 deletions

File tree

server/mergin/sync/public_api_controller.py

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -991,26 +991,42 @@ def push_finish(transaction_id):
991991
)
992992
db.session.add(pv)
993993
db.session.add(project)
994-
db.session.commit()
995994

996-
# let's move uploaded files where they are expected to be
997-
os.renames(files_dir, version_dir)
995+
# move files before committing so a filesystem failure leaves the DB clean
996+
if os.path.exists(files_dir):
997+
os.renames(files_dir, version_dir)
998+
999+
db.session.commit()
9981000

9991001
logging.info(
10001002
f"Push finished for project: {project.id}, project version: {v_next_version}, transaction id: {transaction_id}."
10011003
)
10021004
project_version_created.send(pv)
10031005
push_finished.send(pv)
1004-
except (psycopg2.Error, FileNotFoundError, IntegrityError) as err:
1006+
except (psycopg2.Error, OSError, IntegrityError) as err:
10051007
db.session.rollback()
10061008
logging.exception(
10071009
f"Failed to finish push for project: {project.id}, project version: {v_next_version}, "
10081010
f"transaction id: {transaction_id}.: {str(err)}"
10091011
)
1012+
if (
1013+
os.path.exists(version_dir)
1014+
and not ProjectVersion.query.filter_by(
1015+
project_id=project.id, name=next_version
1016+
).count()
1017+
):
1018+
move_to_tmp(version_dir)
10101019
abort(422, "Failed to create new version: {}".format(str(err)))
10111020
# catch exception during pg transaction so we can rollback and prevent PendingRollbackError during upload clean up
10121021
except gevent.timeout.Timeout:
10131022
db.session.rollback()
1023+
if (
1024+
os.path.exists(version_dir)
1025+
and not ProjectVersion.query.filter_by(
1026+
project_id=project.id, name=next_version
1027+
).count()
1028+
):
1029+
move_to_tmp(version_dir)
10141030
raise
10151031
finally:
10161032
# remove artifacts

server/mergin/sync/public_api_v2_controller.py

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,15 @@ def create_project_version(id):
310310
upload.clear()
311311
return DataSyncError(failed_files=errors).response(422)
312312

313+
if os.path.exists(version_dir):
314+
if ProjectVersion.query.filter_by(
315+
project_id=project.id, name=next_version
316+
).count():
317+
return UploadError(
318+
error=f"Version {v_next_version} already exists"
319+
).response(409)
320+
move_to_tmp(version_dir)
321+
313322
try:
314323
# let's keep upload alive until all work is done so no one else can claim it
315324
with upload.heartbeat(5):
@@ -324,17 +333,18 @@ def create_project_version(id):
324333
)
325334
db.session.add(pv)
326335
db.session.add(project)
327-
db.session.commit()
328336

329-
# let's move uploaded files where they are expected to be
337+
# move files before committing so a filesystem failure leaves the DB clean
330338
if to_be_added_files or to_be_updated_files:
331339
temp_files_dir = os.path.join(
332340
upload.upload_dir, "files", v_next_version
333341
)
334342
os.renames(temp_files_dir, version_dir)
335343

336-
# remove used chunks
337-
# get chunks from added and updated files
344+
db.session.commit()
345+
346+
# remove used chunks only after commit — chunks belong to the now-committed version
347+
if to_be_added_files or to_be_updated_files:
338348
chunks_ids = []
339349
for file in to_be_added_files + to_be_updated_files:
340350
file_chunks = file.get("chunks", [])
@@ -348,7 +358,7 @@ def create_project_version(id):
348358
push_finished.send(pv)
349359
except (
350360
psycopg2.Error,
351-
FileNotFoundError,
361+
OSError,
352362
IntegrityError,
353363
) as err:
354364
db.session.rollback()

0 commit comments

Comments
 (0)