1
0
Fork 0

issue #531: thanks to @guidoreina for bugfix suggestions!

This commit is contained in:
Jan Steemann 2013-05-17 19:29:05 +02:00
parent 613dafe778
commit 78169e3626
4 changed files with 593 additions and 74 deletions

View File

@ -750,17 +750,20 @@ static int RollbackInsert (TRI_document_collection_t* document,
static int RollbackUpdate (TRI_document_collection_t* document,
TRI_doc_mptr_t* newHeader,
TRI_doc_mptr_t* oldHeader) {
TRI_doc_mptr_t* oldHeader,
bool adjustHeader) {
int res;
assert(newHeader != NULL);
assert(oldHeader != NULL);
// ignore any errors we're getting from this
DeleteSecondaryIndexes(document, newHeader, true);
// put back the header into its old position
document->_headers->move(document->_headers, newHeader, oldHeader);
if (adjustHeader) {
// put back the header into its old position
document->_headers->move(document->_headers, newHeader, oldHeader);
}
*newHeader = *oldHeader;
@ -779,7 +782,8 @@ static int RollbackUpdate (TRI_document_collection_t* document,
static int RollbackRemove (TRI_document_collection_t* document,
TRI_doc_mptr_t* newHeader,
TRI_doc_mptr_t* oldHeader) {
TRI_doc_mptr_t* oldHeader,
bool adjustHeader) {
int res;
// there is no new header
@ -794,8 +798,10 @@ static int RollbackRemove (TRI_document_collection_t* document,
LOG_ERROR("error rolling back remove operation");
}
// put back the header into its old position
document->_headers->relink(document->_headers, oldHeader, oldHeader);
if (adjustHeader) {
// put back the header into its old position
document->_headers->relink(document->_headers, oldHeader, oldHeader);
}
return res;
}
@ -851,7 +857,7 @@ static int InsertDocument (TRI_transaction_collection_t* trxCollection,
TRI_doc_mptr_t* mptr,
bool* freeMarker) {
TRI_document_collection_t* document;
bool written;
bool directOperation;
int res;
TRI_ASSERT_MAINTAINER(*freeMarker == true);
@ -893,8 +899,8 @@ static int InsertDocument (TRI_transaction_collection_t* trxCollection,
&marker->base,
totalSize,
forceSync,
&written);
if (! written) {
&directOperation);
if (! directOperation) {
*freeMarker = false;
}
@ -918,11 +924,13 @@ static int InsertDocument (TRI_transaction_collection_t* trxCollection,
idx->postInsert(trxCollection, idx, header);
}
}
return TRI_ERROR_NO_ERROR;
}
else {
// something has failed.... now delete from the indexes again
RollbackInsert(document, header, NULL);
}
// something has failed.... now delete from the indexes again
RollbackInsert(document, header, NULL);
TRI_ASSERT_MAINTAINER(*freeMarker == true);
return res;
}
@ -988,7 +996,7 @@ static int RemoveDocument (TRI_transaction_collection_t* trxCollection,
TRI_primary_collection_t* primary;
TRI_document_collection_t* document;
TRI_doc_mptr_t* header;
bool written;
bool directOperation;
int res;
TRI_ASSERT_MAINTAINER(*freeMarker == true);
@ -1048,22 +1056,24 @@ static int RemoveDocument (TRI_transaction_collection_t* trxCollection,
&marker->base,
totalSize,
forceSync,
&written);
&directOperation);
if (! written) {
if (! directOperation) {
*freeMarker = false;
}
if (res == TRI_ERROR_NO_ERROR) {
if (written) {
if (directOperation) {
// release the header pointer
document->_headers->release(document->_headers, header);
}
return TRI_ERROR_NO_ERROR;
}
else {
// deletion failed. roll back
RollbackRemove(document, NULL, header);
}
// deletion failed. roll back
RollbackRemove(document, NULL, header, ! directOperation);
TRI_ASSERT_MAINTAINER(*freeMarker == true);
return res;
}
@ -1153,7 +1163,7 @@ static int UpdateDocument (TRI_transaction_collection_t* trxCollection,
TRI_doc_mptr_t* newHeader;
TRI_doc_mptr_t oldData;
int res;
bool written;
bool directOperation;
TRI_ASSERT_MAINTAINER(*freeMarker == true);
document = (TRI_document_collection_t*) trxCollection->_collection->_collection;
@ -1215,23 +1225,25 @@ static int UpdateDocument (TRI_transaction_collection_t* trxCollection,
&marker->base,
totalSize,
forceSync,
&written);
&directOperation);
if (! written) {
if (! directOperation) {
*freeMarker = false;
}
if (res == TRI_ERROR_NO_ERROR) {
if (written) {
if (directOperation) {
document->_headers->moveBack(document->_headers, oldHeader);
}
// write new header into result
*mptr = *((TRI_doc_mptr_t*) newHeader);
return TRI_ERROR_NO_ERROR;
}
else {
RollbackUpdate(document, newHeader, &oldData);
}
RollbackUpdate(document, newHeader, &oldData, ! directOperation);
TRI_ASSERT_MAINTAINER(*freeMarker == true);
return res;
}
@ -2988,10 +3000,10 @@ int TRI_RollbackOperationDocumentCollection (TRI_document_collection_t* document
res = RollbackInsert(document, newHeader, NULL);
}
else if (type == TRI_VOC_DOCUMENT_OPERATION_UPDATE) {
res = RollbackUpdate(document, newHeader, oldData);
res = RollbackUpdate(document, newHeader, oldData, true);
}
else if (type == TRI_VOC_DOCUMENT_OPERATION_REMOVE) {
res = RollbackRemove(document, NULL, oldHeader);
res = RollbackRemove(document, NULL, oldHeader, true);
}
else {
res = TRI_ERROR_INTERNAL;
@ -3057,6 +3069,10 @@ int TRI_WriteOperationDocumentCollection (TRI_document_collection_t* document,
bool waitForSync) {
int res;
TRI_DEBUG_INTENTIONAL_FAIL_IF("TRI_WriteOperationDocumentCollection") {
return TRI_ERROR_INTERNAL;
}
if (type == TRI_VOC_DOCUMENT_OPERATION_INSERT) {
res = WriteInsertMarker(document, (TRI_doc_document_key_marker_t*) marker, newHeader, totalSize, waitForSync);
}

View File

@ -234,29 +234,38 @@ static void MoveHeader (TRI_headers_t* h,
if (old->_prev == NULL) {
headers->_begin = header;
}
else if (headers->_begin == header) {
if (header->_next != NULL) {
headers->_begin = header->_next;
}
}
if (old->_next == NULL) {
headers->_end = header;
}
if (header->_prev != NULL && header->_prev == old->_next) {
header->_prev->_next = NULL;
headers->_end = header->_prev;
else if (headers->_end == header) {
if (header->_prev != NULL) {
headers->_end = header->_prev;
}
}
else if (header->_next != NULL && header->_next == old->_prev) {
header->_next->_prev = NULL;
headers->_begin = header->_next;
if (header->_prev != NULL) {
if (header->_prev == old->_next) {
header->_prev->_next = NULL;
}
else {
header->_prev->_next = header->_next;
}
}
/*
if (headers->_begin == old->_next) {
// adjust list start pointer
headers->_begin = header;
if (header->_next != NULL) {
if (header->_next == old->_prev) {
header->_next->_prev = NULL;
}
else {
header->_next->_prev = header->_prev;
}
}
*/
/*
if (old->_next == NULL) {
// adjust list end pointer
headers->_end = header;
}
*/
if (old->_prev != NULL) {
old->_prev->_next = header;
@ -273,11 +282,7 @@ static void MoveHeader (TRI_headers_t* h,
else {
header->_next = NULL;
}
/*
header->_prev = old->_prev;
header->_next = old->_next;
*/
TRI_ASSERT_MAINTAINER(headers->_begin != NULL);
TRI_ASSERT_MAINTAINER(headers->_end != NULL);
TRI_ASSERT_MAINTAINER(header->_prev != header);

View File

@ -491,12 +491,15 @@ static int AddCollectionOperation (TRI_transaction_collection_t* trxCollection,
transaction_operation_t trxOperation;
int res;
TRI_DEBUG_INTENTIONAL_FAIL_IF("AddCollectionOperation-OOM") {
return TRI_ERROR_OUT_OF_MEMORY;
}
if (trxCollection->_operations == NULL) {
res = InitCollectionOperations(trxCollection);
if (res != TRI_ERROR_NO_ERROR) {
return res;
return TRI_ERROR_OUT_OF_MEMORY;
}
}
@ -514,21 +517,23 @@ static int AddCollectionOperation (TRI_transaction_collection_t* trxCollection,
}
res = TRI_PushBackVector(trxCollection->_operations, &trxOperation);
if (res != TRI_ERROR_NO_ERROR) {
return TRI_ERROR_OUT_OF_MEMORY;
}
if (res == TRI_ERROR_NO_ERROR) {
if (type == TRI_VOC_DOCUMENT_OPERATION_UPDATE) {
TRI_document_collection_t* document = (TRI_document_collection_t*) trxCollection->_collection->_collection;
if (type == TRI_VOC_DOCUMENT_OPERATION_UPDATE) {
TRI_document_collection_t* document = (TRI_document_collection_t*) trxCollection->_collection->_collection;
document->_headers->moveBack(document->_headers, oldHeader);
}
else if (type == TRI_VOC_DOCUMENT_OPERATION_REMOVE) {
TRI_document_collection_t* document = (TRI_document_collection_t*) trxCollection->_collection->_collection;
document->_headers->moveBack(document->_headers, oldHeader);
}
else if (type == TRI_VOC_DOCUMENT_OPERATION_REMOVE) {
TRI_document_collection_t* document = (TRI_document_collection_t*) trxCollection->_collection->_collection;
document->_headers->unlink(document->_headers, oldHeader);
}
document->_headers->unlink(document->_headers, oldHeader);
}
return res;
return TRI_ERROR_NO_ERROR;
}
////////////////////////////////////////////////////////////////////////////////
@ -1687,7 +1692,7 @@ int TRI_AddOperationCollectionTransaction (TRI_transaction_collection_t* trxColl
TRI_df_marker_t* marker,
size_t totalSize,
bool syncRequested,
bool* written) {
bool* directOperation) {
TRI_transaction_t* trx;
TRI_primary_collection_t* primary;
int res;
@ -1699,13 +1704,8 @@ int TRI_AddOperationCollectionTransaction (TRI_transaction_collection_t* trxColl
trxCollection->_originalRevision = primary->base._info._tick;
}
// the tick value of a marker must always be greater than the tick value of any other
// existing marker in the collection
TRI_SetRevisionDocumentCollection((TRI_document_collection_t*) primary, marker->_tick);
if (trx->_hints & ((TRI_transaction_hint_t) TRI_TRANSACTION_HINT_SINGLE_OPERATION)) {
// just one operation in the transaction. we can write the marker directly
// TODO: error checking
res = TRI_WriteOperationDocumentCollection((TRI_document_collection_t*) primary,
type,
newHeader,
@ -1714,14 +1714,22 @@ int TRI_AddOperationCollectionTransaction (TRI_transaction_collection_t* trxColl
marker,
totalSize,
syncRequested || trxCollection->_waitForSync || trx->_waitForSync);
*written = true;
*directOperation = true;
}
else {
trx->_hasOperations = true;
res = AddCollectionOperation(trxCollection, type, newHeader, oldHeader, oldData, marker, totalSize);
*written = false;
if (res == TRI_ERROR_NO_ERROR) {
// if everything went well, this will ensure we don't double free etc. headers
*directOperation = false;
}
else {
TRI_ASSERT_MAINTAINER(res == TRI_ERROR_OUT_OF_MEMORY);
// if something went wrong, this will ensure that we'll not manipulate headers twice
*directOperation = true;
}
}
if (syncRequested) {
@ -1732,6 +1740,14 @@ int TRI_AddOperationCollectionTransaction (TRI_transaction_collection_t* trxColl
trx->_waitForSync = true;
}
if (res == TRI_ERROR_NO_ERROR) {
// operation succeeded, now update the revision id for the collection
// the tick value of a marker must always be greater than the tick value of any other
// existing marker in the collection
TRI_SetRevisionDocumentCollection((TRI_document_collection_t*) primary, marker->_tick);
}
return res;
}

View File

@ -2898,6 +2898,483 @@ function transactionCrossCollectionSuite () {
};
}
// -----------------------------------------------------------------------------
// --SECTION-- test suite
// -----------------------------------------------------------------------------
////////////////////////////////////////////////////////////////////////////////
/// @brief test suite
////////////////////////////////////////////////////////////////////////////////
function transactionServerFailuresSuite () {
var cn = "UnitTestsTransaction";
var c = null;
return {
////////////////////////////////////////////////////////////////////////////////
/// @brief set up
////////////////////////////////////////////////////////////////////////////////
setUp : function () {
internal.debugClearFailAt();
db._drop(cn);
},
////////////////////////////////////////////////////////////////////////////////
/// @brief tear down
////////////////////////////////////////////////////////////////////////////////
tearDown : function () {
internal.debugClearFailAt();
if (c !== null) {
c.drop();
}
c = null;
},
////////////////////////////////////////////////////////////////////////////////
/// @brief test: rollback in case of a server-side fail
////////////////////////////////////////////////////////////////////////////////
testRollbackInsertSingle1 : function () {
c = db._create(cn);
internal.debugSetFailAt("TRI_WriteOperationDocumentCollection");
try {
c.save({ _key: "foo" });
fail();
}
catch (err) {
assertEqual(internal.errors.ERROR_INTERNAL.code, err.errorNum);
}
},
////////////////////////////////////////////////////////////////////////////////
/// @brief test: rollback in case of a server-side fail
////////////////////////////////////////////////////////////////////////////////
testRollbackInsertSingle2 : function () {
c = db._create(cn);
c.save({ _key: "foo" });
internal.debugSetFailAt("TRI_WriteOperationDocumentCollection");
try {
c.save({ _key: "foo2" });
fail();
}
catch (err) {
assertEqual(internal.errors.ERROR_INTERNAL.code, err.errorNum);
}
},
////////////////////////////////////////////////////////////////////////////////
/// @brief test: rollback in case of a server-side fail
////////////////////////////////////////////////////////////////////////////////
testRollbackInsertMulti1 : function () {
c = db._create(cn);
c.save({ _key: "baz" });
var obj = {
collections : {
write: [ cn ]
},
action : function () {
c.save({ _key: "foo" });
internal.debugSetFailAt("AddCollectionOperation-OOM");
c.save({ _key: "bar" });
fail();
}
};
try {
TRANSACTION(obj);
fail();
}
catch (err) {
assertEqual(internal.errors.ERROR_OUT_OF_MEMORY.code, err.errorNum);
}
assertEqual(1, c.count());
assertEqual("baz", c.document("baz")._key);
},
////////////////////////////////////////////////////////////////////////////////
/// @brief test: rollback in case of a server-side fail
////////////////////////////////////////////////////////////////////////////////
testRollbackInsertMulti2 : function () {
c = db._create(cn);
var i;
for (i = 0; i < 100; ++i) {
c.save({ _key: "key" + i });
}
var obj = {
collections : {
write: [ cn ]
},
action : function () {
for (i = 0; i < 100; ++i) {
c.save({ _key: "foo" + i });
}
internal.debugSetFailAt("AddCollectionOperation-OOM");
c.save({ _key: "bar" });
fail();
}
};
try {
TRANSACTION(obj);
fail();
}
catch (err) {
assertEqual(internal.errors.ERROR_OUT_OF_MEMORY.code, err.errorNum);
}
assertEqual(100, c.count());
assertEqual("key0", c.document("key0")._key);
assertEqual("key99", c.document("key99")._key);
},
////////////////////////////////////////////////////////////////////////////////
/// @brief test: rollback in case of a server-side fail
////////////////////////////////////////////////////////////////////////////////
testRollbackUpdateSingle1 : function () {
c = db._create(cn);
c.save({ _key: "foo", value: 1 });
internal.debugSetFailAt("TRI_WriteOperationDocumentCollection");
try {
c.update("foo", { value: 2 });
fail();
}
catch (err) {
assertEqual(internal.errors.ERROR_INTERNAL.code, err.errorNum);
}
assertEqual(1, c.count());
assertEqual("foo", c.document("foo")._key);
assertEqual(1, c.document("foo").value);
},
////////////////////////////////////////////////////////////////////////////////
/// @brief test: rollback in case of a server-side fail
////////////////////////////////////////////////////////////////////////////////
testRollbackUpdateSingle2 : function () {
c = db._create(cn);
c.save({ _key: "foo", value: 1 });
c.save({ _key: "bar", value: "a" });
c.update("foo", { value: 2 });
internal.debugSetFailAt("TRI_WriteOperationDocumentCollection");
try {
c.update("bar", { value: "b" });
fail();
}
catch (err) {
assertEqual(internal.errors.ERROR_INTERNAL.code, err.errorNum);
}
assertEqual(2, c.count());
assertEqual("foo", c.document("foo")._key);
assertEqual(2, c.document("foo").value);
assertEqual("bar", c.document("bar")._key);
assertEqual("a", c.document("bar").value);
},
////////////////////////////////////////////////////////////////////////////////
/// @brief test: rollback in case of a server-side fail
////////////////////////////////////////////////////////////////////////////////
testRollbackUpdateMulti1 : function () {
c = db._create(cn);
c.save({ _key: "foo", value: 1 });
c.save({ _key: "bar", value: "a" });
var obj = {
collections : {
write: [ cn ]
},
action : function () {
c.update("foo", { value: 2 });
internal.debugSetFailAt("AddCollectionOperation-OOM");
c.update("bar", { value: "b" });
fail();
}
};
try {
TRANSACTION(obj);
fail();
}
catch (err) {
assertEqual(internal.errors.ERROR_OUT_OF_MEMORY.code, err.errorNum);
}
assertEqual(2, c.count());
assertEqual("foo", c.document("foo")._key);
assertEqual(1, c.document("foo").value);
assertEqual("bar", c.document("bar")._key);
assertEqual("a", c.document("bar").value);
},
////////////////////////////////////////////////////////////////////////////////
/// @brief test: rollback in case of a server-side fail
////////////////////////////////////////////////////////////////////////////////
testRollbackUpdateMulti2 : function () {
c = db._create(cn);
c.save({ _key: "foo", value: 1 });
c.save({ _key: "bar", value: "a" });
var obj = {
collections : {
write: [ cn ]
},
action : function () {
internal.debugSetFailAt("AddCollectionOperation-OOM");
c.update("foo", { value: 2 });
fail();
}
};
try {
TRANSACTION(obj);
fail();
}
catch (err) {
assertEqual(internal.errors.ERROR_OUT_OF_MEMORY.code, err.errorNum);
}
assertEqual(2, c.count());
assertEqual("foo", c.document("foo")._key);
assertEqual(1, c.document("foo").value);
assertEqual("bar", c.document("bar")._key);
assertEqual("a", c.document("bar").value);
},
////////////////////////////////////////////////////////////////////////////////
/// @brief test: rollback in case of a server-side fail
////////////////////////////////////////////////////////////////////////////////
testRollbackRemoveSingle1 : function () {
c = db._create(cn);
c.save({ _key: "foo" });
internal.debugSetFailAt("TRI_WriteOperationDocumentCollection");
try {
c.remove("foo");
fail();
}
catch (err) {
assertEqual(internal.errors.ERROR_INTERNAL.code, err.errorNum);
}
assertEqual(1, c.count());
assertEqual("foo", c.document("foo")._key);
},
////////////////////////////////////////////////////////////////////////////////
/// @brief test: rollback in case of a server-side fail
////////////////////////////////////////////////////////////////////////////////
testRollbackRemoveSingle2 : function () {
c = db._create(cn);
c.save({ _key: "foo" });
c.save({ _key: "bar" });
internal.debugSetFailAt("TRI_WriteOperationDocumentCollection");
try {
c.remove("foo");
fail();
}
catch (err) {
assertEqual(internal.errors.ERROR_INTERNAL.code, err.errorNum);
}
assertEqual(2, c.count());
assertEqual("foo", c.document("foo")._key);
assertEqual("bar", c.document("bar")._key);
},
////////////////////////////////////////////////////////////////////////////////
/// @brief test: rollback in case of a server-side fail
////////////////////////////////////////////////////////////////////////////////
testRollbackRemoveMulti1 : function () {
c = db._create(cn);
c.save({ _key: "foo" });
c.save({ _key: "bar" });
var obj = {
collections : {
write: [ cn ]
},
action : function () {
c.remove("foo");
internal.debugSetFailAt("AddCollectionOperation-OOM");
c.remove("bar");
fail();
}
};
try {
TRANSACTION(obj);
fail();
}
catch (err) {
assertEqual(internal.errors.ERROR_OUT_OF_MEMORY.code, err.errorNum);
}
assertEqual(2, c.count());
assertEqual("foo", c.document("foo")._key);
assertEqual("bar", c.document("bar")._key);
},
////////////////////////////////////////////////////////////////////////////////
/// @brief test: rollback in case of a server-side fail
////////////////////////////////////////////////////////////////////////////////
testRollbackRemoveMulti2 : function () {
c = db._create(cn);
c.save({ _key: "foo" });
c.save({ _key: "bar" });
var obj = {
collections : {
write: [ cn ]
},
action : function () {
internal.debugSetFailAt("AddCollectionOperation-OOM");
c.remove("foo");
fail();
}
};
try {
TRANSACTION(obj);
fail();
}
catch (err) {
assertEqual(internal.errors.ERROR_OUT_OF_MEMORY.code, err.errorNum);
}
assertEqual(2, c.count());
assertEqual("foo", c.document("foo")._key);
assertEqual("bar", c.document("bar")._key);
},
////////////////////////////////////////////////////////////////////////////////
/// @brief test: rollback in case of a server-side fail
////////////////////////////////////////////////////////////////////////////////
testRollbackRemoveMixed1 : function () {
c = db._create(cn);
var i;
for (i = 0; i < 100; ++i) {
c.save({ _key: "key" + i, value: i });
}
var obj = {
collections : {
write: [ cn ]
},
action : function () {
for (i = 0; i < 50; ++i) {
c.remove("key" + i);
}
for (i = 50; i < 100; ++i) {
c.update("key" + i, { value: i - 50 });
}
internal.debugSetFailAt("AddCollectionOperation-OOM");
c.remove("key50");
fail();
}
};
try {
TRANSACTION(obj);
fail();
}
catch (err) {
assertEqual(internal.errors.ERROR_OUT_OF_MEMORY.code, err.errorNum);
}
assertEqual(100, c.count());
},
////////////////////////////////////////////////////////////////////////////////
/// @brief test: rollback in case of a server-side fail
////////////////////////////////////////////////////////////////////////////////
testRollbackRemoveMixed2 : function () {
c = db._create(cn);
c.save({ _key: "foo" });
c.save({ _key: "bar" });
var obj = {
collections : {
write: [ cn ]
},
action : function () {
var i;
for (i = 0; i < 10; ++i) {
c.save({ _key: "key" + i, value: i });
}
for (i = 0; i < 5; ++i) {
c.remove("key" + i);
}
for (i = 5; i < 10; ++i) {
c.update("key" + i, { value: i - 5 });
}
internal.debugSetFailAt("AddCollectionOperation-OOM");
c.remove("key5");
fail();
}
};
try {
TRANSACTION(obj);
fail();
}
catch (err) {
assertEqual(internal.errors.ERROR_OUT_OF_MEMORY.code, err.errorNum);
}
assertEqual(2, c.count());
assertEqual("foo", c.document("foo")._key);
assertEqual("bar", c.document("bar")._key);
}
};
}
// -----------------------------------------------------------------------------
// --SECTION-- main
// -----------------------------------------------------------------------------
@ -2915,6 +3392,11 @@ jsunity.run(transactionRollbackSuite);
jsunity.run(transactionCountSuite);
jsunity.run(transactionCrossCollectionSuite);
// only run this test suite if server-side failures are enabled
if (internal.debugCanUseFailAt()) {
jsunity.run(transactionServerFailuresSuite);
}
return jsunity.done();
// -----------------------------------------------------------------------------