Skip to content

Chunking ng cleanup #5480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ogoffart merged 3 commits into from
Jan 25, 2017
Merged

Chunking ng cleanup #5480

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f6c77fa
ChunkingNG: delete stale chunks if the file was changed locally
ogoffart Jan 20, 2017
a63d970
ChunkingNG: remove stale chunks when cleaning the uploadInfo table
ogoffart Jan 20, 2017
8a70d22
ChunkingNG: Add some tests
ogoffart Jan 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/libsync/propagateuploadng.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ void PropagateUploadFileNG::doStartUpload()
this, SLOT(slotPropfindIterate(QString,QMap<QString,QString>)));
job->start();
return;
} else if (progressInfo._valid) {
// The upload info is stale. remove the stale chunks on the server
_transferId = progressInfo._transferid;
// Fire and forget. Any error will be ignored.
(new DeleteJob(propagator()->account(), chunkUrl(), this))->start();
Copy link
Member

@jturcotte jturcotte Jan 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be a bit cleaner and safer if chunkUrl would take the transferId as parameter (and could be made static) instead of having to set the variable and reset it later.

// startNewUpload will reset the _transferId and the UploadInfo in the db.
}

startNewUpload();
Expand Down
3 changes: 2 additions & 1 deletion test/syncenginetestutils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ class FakeChunkMoveReply : public QNetworkReply
Q_ASSERT(sourceFolder->isDir);
int count = 0;
int size = 0;
char payload = '*';
char payload = '\0';

do {
if (!sourceFolder->children.contains(QString::number(count)))
Expand All @@ -595,6 +595,7 @@ class FakeChunkMoveReply : public QNetworkReply
Q_ASSERT(!x.isDir);
Q_ASSERT(x.size > 0); // There should not be empty chunks
size += x.size;
Q_ASSERT(!payload || payload == x.contentChar);
payload = x.contentChar;
++count;
} while(true);
Expand Down
81 changes: 58 additions & 23 deletions test/testchunkingng.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,36 @@

using namespace OCC;

/* Upload a 1/3 of a file of given size.
* fakeFolder needs to be synchronized */
static void partialUpload(FakeFolder &fakeFolder, const QString &name, int size)
{
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
QCOMPARE(fakeFolder.uploadState().children.count(), 0); // The state should be clean

fakeFolder.localModifier().insert(name, size);
// Abort when the upload is at 1/3
int sizeWhenAbort = -1;
auto con = QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::transmissionProgress,
[&](const ProgressInfo &progress) {
if (progress.completedSize() > (progress.totalSize() /3 )) {
sizeWhenAbort = progress.completedSize();
fakeFolder.syncEngine().abort();
}
});

QVERIFY(!fakeFolder.syncOnce()); // there should have been an error
QObject::disconnect(con);
QVERIFY(sizeWhenAbort > 0);
QVERIFY(sizeWhenAbort < size);

QCOMPARE(fakeFolder.uploadState().children.count(), 1); // the transfer was done with chunking
auto upStateChildren = fakeFolder.uploadState().children.first().children;
QCOMPARE(sizeWhenAbort, std::accumulate(upStateChildren.cbegin(), upStateChildren.cend(), 0,
[](int s, const FileInfo &i) { return s + i.size; }));
}


class TestChunkingNG : public QObject
{
Q_OBJECT
Expand Down Expand Up @@ -40,37 +70,42 @@ private slots:
FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } });
const int size = 300 * 1000 * 1000; // 300 MB
fakeFolder.localModifier().insert("A/a0", size);

// Abort when the upload is at 1/3
int sizeWhenAbort = -1;
auto con = QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::transmissionProgress,
[&](const ProgressInfo &progress) {
if (progress.completedSize() > (progress.totalSize() /3 )) {
sizeWhenAbort = progress.completedSize();
fakeFolder.syncEngine().abort();
}
});

QVERIFY(!fakeFolder.syncOnce()); // there should have been an error
QObject::disconnect(con);
QVERIFY(sizeWhenAbort > 0);
QVERIFY(sizeWhenAbort < size);
QCOMPARE(fakeFolder.uploadState().children.count(), 1); // the transfer was done with chunking
auto upStateChildren = fakeFolder.uploadState().children.first().children;
QCOMPARE(sizeWhenAbort, std::accumulate(upStateChildren.cbegin(), upStateChildren.cend(), 0,
[](int s, const FileInfo &i) { return s + i.size; }));

partialUpload(fakeFolder, "A/a0", size);
QCOMPARE(fakeFolder.uploadState().children.count(), 1);
auto chunkingId = fakeFolder.uploadState().children.first().name;

// Add a fake file to make sure it gets deleted
fakeFolder.uploadState().children.first().insert("10000", size);
QVERIFY(fakeFolder.syncOnce());

QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
QCOMPARE(fakeFolder.currentRemoteState().find("A/a0")->size, size);
// The same chunk id was re-used
QCOMPARE(fakeFolder.uploadState().children.count(), 1);
QCOMPARE(fakeFolder.uploadState().children.first().name, chunkingId);
}

// We modify the file locally after it has been partially uploaded
void testRemoveStale() {

FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } });
const int size = 300 * 1000 * 1000; // 300 MB
partialUpload(fakeFolder, "A/a0", size);
QCOMPARE(fakeFolder.uploadState().children.count(), 1);
auto chunkingId = fakeFolder.uploadState().children.first().name;


fakeFolder.localModifier().setContents("A/a0", 'B');
fakeFolder.localModifier().appendByte("A/a0");

QVERIFY(fakeFolder.syncOnce());

QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
QCOMPARE(fakeFolder.uploadState().children.count(), 1); // The same chunk id was re-used
QCOMPARE(fakeFolder.currentRemoteState().find("A/a0")->size, size);
QCOMPARE(fakeFolder.currentRemoteState().find("A/a0")->size, size + 1);
// A different chunk id was used, and the previous one is removed
QCOMPARE(fakeFolder.uploadState().children.count(), 1);
QVERIFY(fakeFolder.uploadState().children.first().name != chunkingId);
}
};

Expand Down