From 8a4012a96e38f84e9b6dc78506e2e0c062fcbc14 Mon Sep 17 00:00:00 2001 From: Martin Dobias Date: Tue, 28 Apr 2020 22:46:21 +0200 Subject: [PATCH 1/3] Handle correctly the case when push finish fails --- mergin/client_push.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/mergin/client_push.py b/mergin/client_push.py index 715258e0..bfe05115 100644 --- a/mergin/client_push.py +++ b/mergin/client_push.py @@ -210,16 +210,17 @@ def push_project_finalize(job): resp = job.mc.post("/v1/project/push/finish/%s" % job.transaction_id) job.server_resp = json.load(resp) except ClientError as err: - job.mc.post("/v1/project/push/cancel/%s" % job.transaction_id) # server returns various error messages with filename or something generic # it would be better if it returned list of failed files (and reasons) whenever possible - return {'error': str(err)} - - if 'error' in job.server_resp: - #TODO would be good to get some detailed info from server so user could decide what to do with it - # e.g. diff conflicts, basefiles issues, or any other failure - job.mp.log.error("push failed. server response: " + job.server_resp['error']) - raise ClientError(job.server_resp['error']) + job.mp.log.error("--- push finish failed! " + str(err)) + + # if push finish fails, the transaction is not killed, so we + # need to cancel it so it does not block further uploads + job.mp.log.info("canceling the pending transaction...") + resp_cancel = job.mc.post("/v1/project/push/cancel/%s" % job.transaction_id) + server_resp_cancel = json.load(resp_cancel) + job.mp.log.info("cancel response: " + str(server_resp_cancel)) + raise err job.mp.metadata = { 'name': job.project_path, From 45648e4b9e0eb516394a999c0ce8e97b9ab39f27 Mon Sep 17 00:00:00 2001 From: Martin Dobias Date: Tue, 28 Apr 2020 23:38:24 +0200 Subject: [PATCH 2/3] Add logging of checkpointing --- mergin/client_push.py | 2 +- mergin/merginproject.py | 2 +- mergin/utils.py | 10 +++++++++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/mergin/client_push.py b/mergin/client_push.py index bfe05115..71fe613a 100644 --- a/mergin/client_push.py +++ b/mergin/client_push.py @@ -135,7 +135,7 @@ def push_project_async(mc, directory): for file in upload_files: # do checkpoint to push changes from wal file to gpkg if there is no diff if "diff" not in file and mp.is_versioned_file(file["path"]): - do_sqlite_checkpoint(mp.fpath(file["path"])) + do_sqlite_checkpoint(mp.fpath(file["path"]), mp.log) file["checksum"] = generate_checksum(mp.fpath(file["path"])) file['location'] = mp.fpath_meta(file['diff']['path']) if 'diff' in file else mp.fpath(file['path']) diff --git a/mergin/merginproject.py b/mergin/merginproject.py index 1a02de51..cc3126a8 100644 --- a/mergin/merginproject.py +++ b/mergin/merginproject.py @@ -289,7 +289,7 @@ def get_push_changes(self): changes = self.compare_file_sets(self.metadata['files'], self.inspect_files()) # do checkpoint to push changes from wal file to gpkg for file in changes['added'] + changes['updated']: - size, checksum = do_sqlite_checkpoint(self.fpath(file["path"])) + size, checksum = do_sqlite_checkpoint(self.fpath(file["path"]), self.log) if size and checksum: file["size"] = size file["checksum"] = checksum diff --git a/mergin/utils.py b/mergin/utils.py index f7944e66..84045052 100644 --- a/mergin/utils.py +++ b/mergin/utils.py @@ -69,25 +69,33 @@ def int_version(version): return int(version.lstrip('v')) if re.match(r'v\d', version) else None -def do_sqlite_checkpoint(path): +def do_sqlite_checkpoint(path, log=None): """ Function to do checkpoint over the geopackage file which was not able to do diff file. :param path: file's absolute path on disk :type path: str + :param log: optional reference to a logger + :type log: logging.Logger :returns: new size and checksum of file after checkpoint :rtype: int, str """ new_size = None new_checksum = None if ".gpkg" in path and os.path.exists(f'{path}-wal'): + if log: + log.info("checkpoint - going to add it in " + path) conn = sqlite3.connect(path) cursor = conn.cursor() cursor.execute("PRAGMA wal_checkpoint=FULL") + if log: + log.info("checkpoint - return value: " + str(cursor.fetchone())) cursor.execute("VACUUM") conn.commit() conn.close() new_size = os.path.getsize(path) new_checksum = generate_checksum(path) + if log: + log.info("checkpoint - new size {} checksum {}".format(new_size, new_checksum)) return new_size, new_checksum From fc894836136879e26fa079b30d1b5c9f2698eeca Mon Sep 17 00:00:00 2001 From: Martin Dobias Date: Wed, 29 Apr 2020 10:37:55 +0200 Subject: [PATCH 3/3] Removed extra sqlite checkpoint that was causing upload issue --- mergin/client_push.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/mergin/client_push.py b/mergin/client_push.py index 71fe613a..31a64d7a 100644 --- a/mergin/client_push.py +++ b/mergin/client_push.py @@ -133,10 +133,6 @@ def push_project_async(mc, directory): total_size = 0 # prepare file chunks for upload for file in upload_files: - # do checkpoint to push changes from wal file to gpkg if there is no diff - if "diff" not in file and mp.is_versioned_file(file["path"]): - do_sqlite_checkpoint(mp.fpath(file["path"]), mp.log) - file["checksum"] = generate_checksum(mp.fpath(file["path"])) file['location'] = mp.fpath_meta(file['diff']['path']) if 'diff' in file else mp.fpath(file['path']) for chunk_index, chunk_id in enumerate(file["chunks"]):