From ee58cc3b66999abec10200f67316d7fd649f95d5 Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Mon, 11 Apr 2016 11:31:54 +0200 Subject: [PATCH 1/3] SyncEngine: Close the sync journal after the sync run has finished. --- src/libsync/syncengine.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 385521c8d..6365ff6df 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -1018,6 +1018,7 @@ void SyncEngine::finalize(bool success) _thread.wait(); csync_commit(_csync_ctx); + _journal->close(); qDebug() << "CSync run took " << _stopWatch.addLapTime(QLatin1String("Sync Finished")); _stopWatch.stop(); From 648328fbe22383164354b4aae8ab9bfecfa03028 Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Mon, 11 Apr 2016 12:40:19 +0200 Subject: [PATCH 2/3] SyncJournalDb: Close the db on error in getFileRecord() The idea is that the next call to any database operation will try to reopen the database through the checkConnect() method. So even if there was a disconnect trom the db file, this will reestablish the connection. --- src/libsync/syncjournaldb.cpp | 9 +++++++-- src/libsync/syncjournaldb.h | 5 ++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/libsync/syncjournaldb.cpp b/src/libsync/syncjournaldb.cpp index 26cfa6d6c..215bd0072 100644 --- a/src/libsync/syncjournaldb.cpp +++ b/src/libsync/syncjournaldb.cpp @@ -770,7 +770,7 @@ bool SyncJournalDb::deleteFileRecord(const QString& filename, bool recursively) } -SyncJournalFileRecord SyncJournalDb::getFileRecord( const QString& filename ) +SyncJournalFileRecord SyncJournalDb::getFileRecord(const QString& filename) { QMutexLocker locker(&_mutex); @@ -784,6 +784,8 @@ SyncJournalFileRecord SyncJournalDb::getFileRecord( const QString& filename ) if (!_getFileRecordQuery->exec()) { QString err = _getFileRecordQuery->error(); qDebug() << "Error creating prepared statement: " << _getFileRecordQuery->lastQuery() << ", Error:" << err;; + locker.unlock(); + close(); return rec; } @@ -806,7 +808,10 @@ SyncJournalFileRecord SyncJournalDb::getFileRecord( const QString& filename ) } } else { QString err = _getFileRecordQuery->error(); - qDebug() << "No journal entry found for " << filename; + qDebug() << "No journal entry found for " << filename << "Error: " << err; + locker.unlock(); + close(); + locker.relock(); } _getFileRecordQuery->reset_and_clear_bindings(); } diff --git a/src/libsync/syncjournaldb.h b/src/libsync/syncjournaldb.h index 29cc2a995..c6b40bf4a 100644 --- a/src/libsync/syncjournaldb.h +++ b/src/libsync/syncjournaldb.h @@ -38,7 +38,10 @@ class OWNCLOUDSYNC_EXPORT SyncJournalDb : public QObject public: explicit SyncJournalDb(const QString& path, QObject *parent = 0); virtual ~SyncJournalDb(); - SyncJournalFileRecord getFileRecord( const QString& filename ); + + // to verify that the record could be queried successfully check + // with SyncJournalFileRecord::isValid() + SyncJournalFileRecord getFileRecord(const QString& filename); bool setFileRecord( const SyncJournalFileRecord& record ); /// Like setFileRecord, but preserves checksums From d433c24186c607e73c0493be1aef1a44d451092d Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Mon, 11 Apr 2016 12:41:26 +0200 Subject: [PATCH 3/3] Check if the record returned from getFileRecord is valid. Handle database fails properly. --- src/libsync/owncloudpropagator.cpp | 12 +++++++----- src/libsync/propagateremotemove.cpp | 16 +++++++++++----- src/libsync/propagatorjobs.cpp | 6 ++++-- src/libsync/syncengine.cpp | 2 +- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 8ab63993f..341cd8000 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -754,11 +754,13 @@ void CleanupPollsJob::start() auto info = _pollInfos.first(); _pollInfos.pop_front(); - SyncFileItemPtr item(new SyncFileItem( - _journal->getFileRecord(info._file).toSyncFileItem())); - PollJob *job = new PollJob(_account, info._url, item, _journal, _localPath, this); - connect(job, SIGNAL(finishedSignal()), SLOT(slotPollFinished())); - job->start(); + SyncJournalFileRecord record = _journal->getFileRecord(info._file); + SyncFileItemPtr item(new SyncFileItem(record.toSyncFileItem())); + if (record.isValid()) { + PollJob *job = new PollJob(_account, info._url, item, _journal, _localPath, this); + connect(job, SIGNAL(finishedSignal()), SLOT(slotPollFinished())); + job->start(); + } } void CleanupPollsJob::slotPollFinished() diff --git a/src/libsync/propagateremotemove.cpp b/src/libsync/propagateremotemove.cpp index 718b337f6..98f4e7edc 100644 --- a/src/libsync/propagateremotemove.cpp +++ b/src/libsync/propagateremotemove.cpp @@ -154,15 +154,21 @@ void PropagateRemoteMove::finalize() { SyncJournalFileRecord oldRecord = _propagator->_journal->getFileRecord(_item->_originalFile); + // if reading from db failed still continue hoping that deleteFileRecord + // reopens the db successfully. + // The db is only queried to transfer the content checksum from the old + // to the new record. It is not a problem to skip it here. _propagator->_journal->deleteFileRecord(_item->_originalFile); SyncJournalFileRecord record(*_item, _propagator->getFilePath(_item->_renameTarget)); record._path = _item->_renameTarget; - record._contentChecksum = oldRecord._contentChecksum; - record._contentChecksumType = oldRecord._contentChecksumType; - if (record._fileSize != oldRecord._fileSize) { - qDebug() << "Warning: file sizes differ on server vs csync_journal: " << record._fileSize << oldRecord._fileSize; - record._fileSize = oldRecord._fileSize; // server might have claimed different size, we take the old one from the DB + if (oldRecord.isValid()) { + record._contentChecksum = oldRecord._contentChecksum; + record._contentChecksumType = oldRecord._contentChecksumType; + if (record._fileSize != oldRecord._fileSize) { + qDebug() << "Warning: file sizes differ on server vs csync_journal: " << record._fileSize << oldRecord._fileSize; + record._fileSize = oldRecord._fileSize; // server might have claimed different size, we take the old one from the DB + } } _propagator->_journal->setFileRecord(record); diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index a133b6703..f137fb611 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -232,8 +232,10 @@ void PropagateLocalRename::start() SyncJournalFileRecord record(*_item, targetFile); record._path = _item->_renameTarget; - record._contentChecksum = oldRecord._contentChecksum; - record._contentChecksumType = oldRecord._contentChecksumType; + if (oldRecord.isValid()) { + record._contentChecksum = oldRecord._contentChecksum; + record._contentChecksumType = oldRecord._contentChecksumType; + } if (!_item->_isDirectory) { // Directories are saved at the end _propagator->_journal->setFileRecord(record); diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 6365ff6df..a826d7840 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -519,7 +519,7 @@ int SyncEngine::treewalkFile( TREE_WALK_FILE *file, bool remote ) // If the 'W' remote permission changed, update the local filesystem SyncJournalFileRecord prev = _journal->getFileRecord(item->_file); - if (prev._remotePerm.contains('W') != item->_remotePerm.contains('W')) { + if (prev.isValid() && prev._remotePerm.contains('W') != item->_remotePerm.contains('W')) { const bool isReadOnly = !item->_remotePerm.contains('W'); FileSystem::setFileReadOnlyWeak(filePath, isReadOnly); }