Skip to content

Commit ff4b3d1

Browse files
committed
Upload: Store the size in the UploadInfo, and compare it when resolving potential conflict
This is about the conflicts that happens when the file has been uploaded correctly to the server, but the etag was not recieved because the connection was closed before we got the reply. We used to compare only the mtime when comparing the uploaded file and the existing file. However, to be perfectly correct, we also should check the size. This was found because TestChunkingNG::connectionDroppedBeforeEtagRecieved is flaky. Example of faillure found in https://drone.owncloud.com/owncloud/client/481/5 while testing PR #6626 (very trimmed log:) 06-29 07:58:02:015 [ info sync.csync.csync ]: ## Starting local discovery ## 06-29 07:58:02:016 [ info sync.csync.updater ]: Database entry found, compare: 1530259082 <-> 1530259051, etag: <-> 1644a8c8750, inode: 1935629 <-> 1935629, size: 301 <-> 300, perms: 0 <-> ff, type: 0 <-> 0, checksum: <-> SHA1:cc9adedebe27a6259efb8d6ed09f4f2eff559ad1, ignore: 0 06-29 07:58:02:016 [ info sync.csync.updater ]: file: A/a0, instruction: INSTRUCTION_EVAL <<= 06-29 07:58:02:972 [ warning sync.networkjob ]: QNetworkReply::NetworkError(OperationCanceledError) "Connection timed out" QVariant(Invalid) .. next sync... 06-29 07:58:02:980 [ info sync.engine ]: #### Discovery start #################################################### 06-29 07:58:02:981 [ info sync.csync.csync ]: ## Starting local discovery ## 06-29 07:58:02:983 [ info sync.csync.updater ]: Database entry found, compare: 1530259082 <-> 1530259051, etag: <-> 1644a8c8750, inode: 1935629 <-> 1935629, size: 302 <-> 300, perms: 0 <-> ff, type: 0 <-> 0, checksum: <-> SHA1:cc9adedebe27a6259efb8d6ed09f4f2eff559ad1, ignore: 0 06-29 07:58:02:983 [ info sync.csync.updater ]: file: A/a0, instruction: INSTRUCTION_EVAL <<= 06-29 07:58:02:985 [ info sync.csync.csync ]: ## Starting remote discovery ## 06-29 07:58:02:985 [ info sync.networkjob ]: OCC::LsColJob created for "http://localhost/owncloud" + "" "OCC::DiscoverySingleDirectoryJob" 06-29 07:58:02:987 [ info sync.csync.updater ]: Database entry found, compare: 1530259082 <-> 1530259051, etag: 1644a8c8b26 <-> 1644a8c8750, inode: 0 <-> 1935629, size: 301 <-> 300, perms: ff <-> ff, type: 0 <-> 0, checksum: SHA1:5adcdac9608ae0811247f07f4cf1ab0a2ef99154 <-> SHA1:cc9adedebe27a6259efb8d6ed09f4f2eff559ad1, ignore: 0 06-29 07:58:02:987 [ info sync.csync.updater ]: file: A/a0, instruction: INSTRUCTION_EVAL <<= 06-29 07:58:02:989 [ info sync.csync.csync ]: Update detection for remote replica took 0.004 seconds walking 13 files 06-29 07:58:02:990 [ info sync.engine ]: #### Discovery end #################################################### 9 ms 06-29 07:58:02:990 [ info sync.database ]: Updating file record for path: "A/a0" inode: 1935629 modtime: 1530259082 type: 0 etag: "1644a8c8b26" fileId: "16383ea4" remotePerm: "WDNVCKR" fileSize: 301 checksum: "SHA1:cc9adedebe27a6259efb8d6ed09f4f2eff559ad1" 06-29 07:58:02:990 [ info sync.csync.reconciler ]: INSTRUCTION_UPDATE_METADATA client file: A/a0 06-29 07:58:02:990 [ info sync.csync.csync ]: Reconciliation for local replica took 0 seconds visiting 13 files. 06-29 07:58:02:990 [ info sync.csync.reconciler ]: INSTRUCTION_UPDATE_METADATA server dir: A 06-29 07:58:02:990 [ info sync.csync.csync ]: Reconciliation for remote replica took 0 seconds visiting 13 files. 06-29 07:58:02:990 [ info sync.engine ]: #### Reconcile end #################################################### 9 ms 06-29 07:58:02:990 [ info sync.database ]: Updating local metadata for: "A/a0" 1530259082 302 1935629 FAIL! : TestChunkingNG::connectionDroppedBeforeEtagRecieved(small file) '!fakeFolder.syncOnce()' returned FALSE. ()
1 parent 3e40166 commit ff4b3d1

File tree

5 files changed

+10
-4
lines changed

5 files changed

+10
-4
lines changed

src/common/syncjournaldb.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject
108108
}
109109
int _chunk;
110110
int _transferid;
111-
quint64 _size; //currently unused
111+
qint64 _size;
112112
qint64 _modtime;
113113
int _errorCount;
114114
bool _valid;

src/csync/csync_reconcile.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,8 @@ static void _csync_merge_algorithm_visitor(csync_file_stat_t *cur, CSYNC * ctx)
370370
auto remoteNode = ctx->current == REMOTE_REPLICA ? cur : other;
371371
auto localNode = ctx->current == REMOTE_REPLICA ? other : cur;
372372
remoteNode->instruction = CSYNC_INSTRUCTION_NONE;
373-
localNode->instruction = up._modtime == localNode->modtime ? CSYNC_INSTRUCTION_UPDATE_METADATA : CSYNC_INSTRUCTION_SYNC;
373+
localNode->instruction = up._modtime == localNode->modtime && up._size == localNode->size ?
374+
CSYNC_INSTRUCTION_UPDATE_METADATA : CSYNC_INSTRUCTION_SYNC;
374375

375376
// Update the etag and other server metadata in the journal already
376377
// (We can't use a typical CSYNC_INSTRUCTION_UPDATE_METADATA because

src/libsync/propagateuploadng.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@ void PropagateUploadFileNG::doStartUpload()
8383
propagator()->_activeJobList.append(this);
8484

8585
const SyncJournalDb::UploadInfo progressInfo = propagator()->_journal->getUploadInfo(_item->_file);
86-
if (progressInfo._valid && progressInfo.isChunked() && progressInfo._modtime == _item->_modtime) {
86+
if (progressInfo._valid && progressInfo.isChunked() && progressInfo._modtime == _item->_modtime
87+
&& progressInfo._size == qint64(_item->_size)) {
8788
_transferId = progressInfo._transferid;
8889
auto url = chunkUrl();
8990
auto job = new LsColJob(propagator()->account(), url, this);
@@ -239,6 +240,7 @@ void PropagateUploadFileNG::startNewUpload()
239240
pi._transferid = _transferId;
240241
pi._modtime = _item->_modtime;
241242
pi._contentChecksum = _item->_checksumHeader;
243+
pi._size = _item->_size;
242244
propagator()->_journal->setUploadInfo(_item->_file, pi);
243245
propagator()->_journal->commit("Upload info");
244246
QMap<QByteArray, QByteArray> headers;

src/libsync/propagateuploadv1.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ void PropagateUploadFileV1::doStartUpload()
4343

4444
const SyncJournalDb::UploadInfo progressInfo = propagator()->_journal->getUploadInfo(_item->_file);
4545

46-
if (progressInfo._valid && progressInfo.isChunked() && progressInfo._modtime == _item->_modtime
46+
if (progressInfo._valid && progressInfo.isChunked() && progressInfo._modtime == _item->_modtime && progressInfo._size == qint64(_item->_size)
4747
&& (progressInfo._contentChecksum == _item->_checksumHeader || progressInfo._contentChecksum.isEmpty() || _item->_checksumHeader.isEmpty())) {
4848
_startChunk = progressInfo._chunk;
4949
_transferId = progressInfo._transferid;
@@ -59,6 +59,7 @@ void PropagateUploadFileV1::doStartUpload()
5959
pi._modtime = _item->_modtime;
6060
pi._errorCount = 0;
6161
pi._contentChecksum = _item->_checksumHeader;
62+
pi._size = _item->_size;
6263
propagator()->_journal->setUploadInfo(_item->_file, pi);
6364
propagator()->_journal->commit("Upload info");
6465
}
@@ -285,6 +286,7 @@ void PropagateUploadFileV1::slotPutFinished()
285286
pi._modtime = _item->_modtime;
286287
pi._errorCount = 0; // successful chunk upload resets
287288
pi._contentChecksum = _item->_checksumHeader;
289+
pi._size = _item->_size;
288290
propagator()->_journal->setUploadInfo(_item->_file, pi);
289291
propagator()->_journal->commit("Upload info");
290292
startNextChunk();

test/testuploadreset.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ private slots:
3636
uploadInfo._transferid = 1;
3737
uploadInfo._valid = true;
3838
uploadInfo._modtime = Utility::qDateTimeToTime_t(modTime);
39+
uploadInfo._size = size;
3940
fakeFolder.syncEngine().journal()->setUploadInfo("A/a0", uploadInfo);
4041

4142
fakeFolder.uploadState().mkdir("1");

0 commit comments

Comments
 (0)