Skip to content

196 objects not cleaned up on transaction failure #306

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
merged 4 commits into from
Aug 13, 2025
+219 −51
Merged

196 objects not cleaned up on transaction failure #306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
efa1a9d
Include S3 transaction rollback test
StevenRojasC Aug 7, 2025
6824f6b
Implement S3 transaction rollback for PMGD handler
StevenRojasC Aug 7, 2025
da28049
Implement S3 transaction rollback for Neo4j handler
StevenRojasC Aug 7, 2025
33dd7a3
Automated coverage update
github-actions[bot] Aug 9, 2025
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
12 changes: 6 additions & 6 deletions .github/coverage/cpp.develop.coverage_report.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ src/CommunicationManager.cc 53 32 60% 50,65-66,68-71
src/DescriptorsCommand.cc 733 455 62% 96,102,115,168-170,174,188-192,244-255,259-260,264-269,279,287-290,304-306,310,325-334,336,356-357,364-368,401,404-407,413,415-416,419-421,423-424,426-427,430-431,433-435,458-460,464-468,475,481,483,485-486,489,523,562-564,574,584-589,595,597,599-600,602-603,643,671,691,695,697,700,702-705,708-713,715-719,721,724,726,728,731,734-735,737,739,741-746,749,751-753,756-757,759,761,763-764,766,768,770-772,774,776-781,786-787,790,792,794,819-821,840-841,844-845,858-861,876,878,881-882,885,915-917,934-936,964-966,972-973,1003-1006,1022-1023,1025-1026,1028,1030-1031,1033,1048,1051,1059,1064,1085-1087,1091-1094,1103,1105,1107-1111,1114-1115,1118-1121,1123,1125-1132,1157-1161,1166-1170,1180-1183,1197-1200,1212-1213,1254-1257,1287,1290,1310-1314,1330-1331
src/DescriptorsManager.cc 25 23 92% 49-50
src/ExceptionsCommand.cc 6 0 0% 35-40
src/ImageCommand.cc 308 143 46% 66,73-80,82-87,90-91,93-94,98-99,107-108,110,117,120,122-123,125-126,128-129,131-132,135,162,173-174,185-186,188,193-194,204-205,207,220-222,228-236,238-240,253-254,264-274,276-277,279-280,285,293,304,311,315,318,320,322,344,346-347,350-355,357,359,381-383,386-388,392-395,401,403,410-413,427,434,440-441,445-446,457-460,463-468,473-475,486-489,494-498,503-504,506-507,509-513,516,518-522,525-528,531-532,535,537,542
src/ImageCommand.cc 309 153 49% 66,73-80,82-87,90-91,93-94,98-99,107-108,110,117,120,122-123,125-126,128-129,131-132,135,162,173-174,185-186,188,204-205,207,220-222,228-236,238-240,253-254,264-274,276-277,279-280,285,293,304,311,315,318,320,322,344,346-347,350-355,357,359,381-383,392-395,401,403,427,434,440-441,445-446,457-460,463-468,473-475,486-489,494-498,503-504,506-507,509-513,516,518-522,525-528,531-532,535,537,542
src/ImageLoop.cc 253 232 91% 63,130,182-185,215,221,265,285,288,297-298,300,307-308,322-323,330,334,338
src/Neo4jBaseCommands.cc 36 23 63% 17,21,23-24,26,33,53,57-59,66-68
src/Neo4JDescriptorCommands.cc 551 405 73% 90,96-99,101-102,104-105,107,110-111,113,116-117,135-136,138-139,141,143-144,146,151,154-155,158-161,163-164,168,170-173,175-178,231,234,313-324,329-331,355-357,361-362,376-385,387,404-408,510,513-516,539-541,545-550,556-557,614-616,628-633,693,738-740,745-746,749-750,765-768,815-817,847-849,856-857,876-879,904-906,910-913,975-976,1028-1029,1032-1033,1054-1058
Expand All @@ -24,10 +24,10 @@ src/PmgdFilter.cc 101 87 86% 76,80,91-93,10
src/PMGDIterators.cc 52 44 84% 62,76,96-101
src/PMGDQuery.cc 466 370 79% 94-96,129,131,135,140,143-144,167-169,171-172,211-212,216-218,248,254,258,288,298,302-305,307,309,311,317,321-322,354,356,358,360-361,364-373,375-377,379,383-384,386-388,409,412-414,419-424,446,449-450,480-481,492,547,549-557,653-654,658-662,664-668
src/PMGDQueryHandler.cc 649 540 83% 88-90,172-173,175-176,214-215,227-229,236,285,287,291,296,298,326-327,344,346,350,352,403-404,406,408-413,415,417,469-470,484-485,494,496,502,504,530-532,543,572,611,613,618-619,655,657-661,683-685,687-692,694,726,735-736,743,747-749,751,754,757,821,828,856,876-882,884-885,887,889,901-902,921-923,927-928,971,1018-1019,1021-1023,1044,1056,1058-1059
src/QueryHandlerBase.cc 34 12 35% 27-29,35-36,46,48-49,53,57-59,61-63,65-67,69-70,73-74
src/QueryHandlerBase.cc 35 15 42% 29,35-36,46,48-49,53,57-59,61-63,65-67,69-70,73-74
src/QueryHandlerExample.cc 33 18 54% 65-67,75-78,84-85,89-92,94-95
src/QueryHandlerNeo4j.cc 159 96 60% 87-89,97-100,111-112,116-120,123-127,131-138,141-144,146-148,157-159,163-165,205,216-218,239-240,257-259,264-266,281-288,292-294
src/QueryHandlerPMGD.cc 350 290 82% 114-116,124-127,150-151,157-158,188-190,194-196,214-215,238-244,248-250,345-346,398-399,408,420-427,429-431,438-446,483,485,540-541,543-544
src/QueryHandlerNeo4j.cc 166 98 59% 87-89,97-100,111-112,116-120,123-127,131-138,141-144,146-148,157-159,163-165,208,219-221,224,245-246,252-255,269-271,276-278,293-300,304-306
src/QueryHandlerPMGD.cc 357 296 82% 115-117,125-128,151-152,158-159,189-191,195-197,215-216,239-245,249-251,287,356-357,409-410,419,431-438,440-442,449-457,494,496,551-552,554-555
src/QueryMessage.cc 12 3 25% 42-43,45-46,48,51-54
src/RSCommand.cc 144 105 72% 65-67,73-74,98,100-101,103,110,131,134-138,141,172-174,176,178-181,188,262,285,287-289,291-297,301
src/SearchExpression.cc 99 38 38% 59,132-133,135,137-139,143,146,148-153,157,160,168-170,177,180-181,183-185,188,192-195,197,201,217-222,224-225,227,235-240,243,247-249,252-256,263,276,284-285
Expand All @@ -41,7 +41,7 @@ src/vcl/Exception.cc 7 6 85% 38
src/vcl/FaissDescriptorSet.cc 212 183 86% 83,115-116,132,167,187-188,204-205,224-225,238-239,245,258-259,261,272-273,279,305-306,308-309,311,374-375,381,405
src/vcl/Filter.cc 132 107 81% 92-93,108-109,124-126,140-141,147-148,190-191,196-197,204-205,218-219,225-226,233,242,278,285
src/vcl/FlinngDescriptorSet.cc 149 109 73% 60-66,89,109-111,113-114,118-121,124,126,128,130,132,134-137,140-141,143-144,170-171,176-177,182,206,208,228,248,279
src/vcl/Image.cc 926 694 74% 62,73-74,76-78,81-84,86,92,101,122-123,125,132-133,135,147,165,170,193,196-199,223,246,249-252,264,273,276-279,291,323,326-329,341,347,349-352,360-362,369,393-396,415,417,425,427,432,436,441,445,459,462,467-468,471-472,474,490,500,513,531,553-556,566,597,603-605,614-615,617,625,629,634,637-648,676-678,730,775-776,827,856-860,862-868,870,872-873,914,917-918,957-958,962-963,984,1003-1004,1006,1046-1048,1050-1054,1056-1060,1062-1066,1068-1072,1074-1078,1080-1083,1106,1127,1146-1154,1165-1166,1185-1204,1216-1217,1225,1236,1238-1240,1242-1244,1246,1264-1265,1267,1272-1273,1275,1296,1300,1303,1310,1325,1333,1342,1356,1381,1399,1482,1501
src/vcl/Image.cc 927 697 75% 62,73-74,76-78,81-84,86,92,101,122-123,125,132-133,135,147,165,170,193,196-199,223,246,249-252,264,273,276-279,291,323,326-329,341,347,349-352,360-362,369,393-396,415,417,425,427,432,436,441,445,459,462,467-468,471-472,474,490,500,513,531,553-556,566,597,603-605,614-615,617,625,629,634,637-648,676-678,730,775-776,827,856-860,862-868,870,872-873,914,917-918,957-958,984,1003-1004,1006,1046-1048,1050-1054,1056-1060,1062-1066,1068-1072,1074-1078,1080-1083,1106,1127,1146-1154,1165-1166,1185-1204,1216-1217,1225,1236,1238-1240,1242-1244,1246,1264-1265,1267,1272-1273,1275,1296,1300,1303,1310,1325,1333,1342,1356,1381,1399,1482,1501
src/vcl/KeyFrame.cc 303 244 80% 58,62,86,90,95,97,102,105-107,109-111,113,119,139,148,154,172,186,190,216,220,224,235,239,249,255,274,284,288,307,315,341,345,347,359,367,369,394,396,405,430,442,449,465,469,478,483,495,500,507,514,518,525,541,547,557,563
src/vcl/RemoteConnection.cc 295 172 58% 56-59,66-69,82,87,91-94,96,120-122,131-134,150-153,155,169-172,174,186-189,191,204-207,209,221-224,226,241-244,247,255-257,259-262,264,273-274,285-286,295-299,306-308,310,329-332,339-341,343,355-358,370-372,374,402-404,406,418-421,433-435,437,454-456,467-469,471,484-487,492-494,496,499,502-504,506-507,509
src/vcl/TDBDenseDescriptorSet.cc 115 111 96% 95,162-163,213
Expand All @@ -64,5 +64,5 @@ utils/src/kubernetes/KubeHelper.cc 183 37 20% 23-30,32-35,37
utils/src/stats/SystemStats.cc 250 249 99% 455
utils/src/timers/TimerMap.cc 82 74 90% 126,151,153,155-158,162
------------------------------------------------------------------------------
TOTAL 11689 8338 71%
TOTAL 11706 8362 71%
------------------------------------------------------------------------------
2 changes: 1 addition & 1 deletion .github/coverage/cpp.develop.coverage_value.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
71.332
71.4335
96 changes: 54 additions & 42 deletions src/QueryHandlerNeo4j.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,63 +186,75 @@ void QueryHandlerNeo4j::process_query(protobufs::queryMessage &proto_query,
Json::Value json_responses;
Json::Value cmd_result;

std::vector<std::string> images_log;

Json::Value root;
int blob_count = 0;
bool error = false;

rc = parse_commands(proto_query, root);
// begin neo4j transaction
tx = neoconn_pool->open_tx(conn, 10000, "w");
for (int j = 0; j < root.size(); j++) {
Json::Value neo4j_resp;
std::string cypher;
try {
rc = parse_commands(proto_query, root);
// begin neo4j transaction
tx = neoconn_pool->open_tx(conn, 10000, "w");
for (int j = 0; j < root.size(); j++) {
Json::Value neo4j_resp;
std::string cypher;

Json::Value &query = root[j];
std::string cmd = query.getMemberNames()[0];
Json::Value &query = root[j];
std::string cmd = query.getMemberNames()[0];


if (_rs_cmds.count(cmd) == 0) {
std::cout<<"Command: " << cmd << "Does not exist!" << std::endl;
}
if (_rs_cmds.count(cmd) == 0) {
std::cout<<"Command: " << cmd << "Does not exist!" << std::endl;
}

Neo4jCommand *rscmd = _rs_cmds[cmd];
cypher = query[cmd]["cypher"].asString();
Neo4jCommand *rscmd = _rs_cmds[cmd];
cypher = query[cmd]["cypher"].asString();

const std::string &blob =
rscmd->need_blob(query) ? proto_query.blobs(blob_count++) : "";
const std::string &blob =
rscmd->need_blob(query) ? proto_query.blobs(blob_count++) : "";

rc = rscmd->data_processing(cypher, query, blob, 0, cmd_result);
if (rc != 0) {
error = true;
proto_res.set_json(fastWriter.write(cmd_result));
break;
}
res_stream = neoconn_pool->run_in_tx((char *)cypher.c_str(), tx);
neo4j_resp = neoconn_pool->results_to_json(res_stream);
query["cp_result"] = cmd_result;
Json::Value resp_retval = rscmd->construct_responses(neo4j_resp, query, proto_res, blob);
//THIS IS VERY CLUNKY and confusing, NEEDS TO BE REFACTORED
if (neo4j_resp.isMember("metadata_res") && (cmd == "NeoAdd" || cmd == "NeoFind")) {
resp_retval["metadata_res"] = neo4j_resp["metadata_res"];
}
json_responses.append(resp_retval);
rc = rscmd->data_processing(cypher, query, blob, 0, cmd_result);
if (rc != 0) {
error = true;
proto_res.set_json(fastWriter.write(cmd_result));
break;
}
if (cmd_result.isMember("image_added")) {
images_log.push_back(cmd_result["image_added"].asString());
}
res_stream = neoconn_pool->run_in_tx((char *)cypher.c_str(), tx);
neo4j_resp = neoconn_pool->results_to_json(res_stream);
query["cp_result"] = cmd_result;
Json::Value resp_retval = rscmd->construct_responses(neo4j_resp, query, proto_res, blob);
//THIS IS VERY CLUNKY and confusing, NEEDS TO BE REFACTORED
if (neo4j_resp.isMember("metadata_res") && (cmd == "NeoAdd" || cmd == "NeoFind")) {
resp_retval["metadata_res"] = neo4j_resp["metadata_res"];
}
json_responses.append(resp_retval);


}
proto_res.set_json(fastWriter.write(json_responses));
// commit neo4j transaction, needs to be updated in future to account for
// errors on response construction
if (error == false) {
rc = neoconn_pool->commit_tx(tx);

if(rc != 0){
printf("Warning! Transaction Error: %d\n", rc);
exit(1);
}
proto_res.set_json(fastWriter.write(json_responses));
// commit neo4j transaction, needs to be updated in future to account for
// errors on response construction
if (error == false) {
rc = neoconn_pool->commit_tx(tx);

if(rc != 0){
printf("Warning! Transaction Error: %d\n", rc);
exit(1);
}

}

neoconn_pool->put_conn(conn);
} catch(...) {
VCL::RemoteConnection *connection = get_existing_connection();
for (const std::string image : images_log) {
connection->Remove_Object(image);
}
}

neoconn_pool->put_conn(conn);
}

int QueryHandlerNeo4j::parse_commands(
Expand Down
11 changes: 11 additions & 0 deletions src/QueryHandlerPMGD.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include "pmgd.h"
#include "util.h"
#include "PmgdFilter.h"
#include "OpsIOCoordinator.h"

#include "APISchema.h"
#include <jsoncpp/json/writer.h>
Expand Down Expand Up @@ -276,6 +277,16 @@ void QueryHandlerPMGD::process_query(
Json::Value response;
response.append(exception_error);
proto_res.set_json(fastWriter.write(response));

if (VDMSConfig::instance()->get_aws_flag()) {
VCL::RemoteConnection *connection = get_existing_connection();
for (const std::string image : images_log) {
connection->Remove_Object(image);
}
for (const std::string video : videos_log) {
connection->Remove_Object(video);
}
}
};

try {
Expand Down
149 changes: 147 additions & 2 deletions tests/unit_tests/RemoteConnection_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,9 +499,154 @@ TEST_F(RemoteConnectionTest, ImageAddCropFailure) {
EXPECT_EQ(json_response[0]["status"].asString(), "-1");
EXPECT_EQ(json_response[0]["info"].asString(), "Internal Server Error: VCL Exception at QH\n");

std::string dbname = VDMSConfig::instance()->get_path_pmgd();
std::filesystem::remove_all(dbname.c_str());
} catch (...) {
printErrorMessage("ImageAddCropFailure");
}
}

TEST_F(RemoteConnectionTest, ImageTransactionRollback) {
try {
int s3_num_objects;
const char *s3_num_objects_cmd = "mc ls --recursive myminio/minio-bucket | wc -l";
std::array<char, 8> buffer;
std::string result;

std::string string_query_simple_add_image("[ \
{ \
\"AddImage\": { \
\"properties\": { \
\"name\": \"SampleImage\" \
}, \
\"format\": \"png\" \
} \
} \
]");

std::string string_query_image_rollback("[");
string_query_image_rollback += " \
{ \
\"AddImage\": { \
\"properties\": { \
\"name\": \"ImageTransactionRollback_1\" \
}, \
\"format\": \"png\" \
} \
}, \
{ \
\"AddImage\": { \
\"operations\": [{ \
\"type\": \"crop\", \
\"x\": 250, \
\"y\": 250, \
\"width\": 100, \
\"height\": 100 \
}], \
\"properties\": { \
\"name\": \"ImageTransactionRollback_2\" \
}, \
\"format\": \"png\" \
} \
} \
";
string_query_image_rollback += "]";

VDMS::Server VDMS_server("unit_tests/config-aws-tests.json", "", "", "");

QueryHandlerPMGD query_handler;
query_handler.reset_autodelete_init_flag(); // set flag to show autodelete queue has
// been initialized

VDMS::protobufs::queryMessage proto_query;
proto_query.set_json(string_query_simple_add_image);

std::string image;
std::ifstream file("test_images/brain.png",
std::ios::in | std::ios::binary | std::ios::ate);

image.resize(file.tellg());

file.seekg(0, std::ios::beg);
if (!file.read(&image[0], image.size()))
std::cout << "error" << std::endl;

proto_query.add_blobs(image);

VDMS::protobufs::queryMessage response;
query_handler.process_query(proto_query, response);

// Get initial number of objects stored in S3
std::unique_ptr<FILE, decltype(&pclose)> pipe1(popen(s3_num_objects_cmd, "r"), pclose);
if (!pipe1) {
throw std::runtime_error("popen() failed!");
}
while (fgets(buffer.data(), static_cast<int>(buffer.size()), pipe1.get()) != nullptr) {
result += buffer.data();
}
s3_num_objects = stoi(result);
result.clear();

proto_query.clear_blobs();
proto_query.set_json(string_query_image_rollback);
proto_query.add_blobs(image);
proto_query.add_blobs(image);
query_handler.process_query(proto_query, response);

Json::Reader json_reader;
Json::Value json_response;

json_reader.parse(response.json(), json_response);

EXPECT_EQ(json_response[0]["status"].asString(), "-1");
EXPECT_EQ(json_response[0]["info"].asString(), "Internal Server Error: VCL Exception at QH\n");

std::string string_query_image_lookup("[");
string_query_image_lookup += " \
{ \
\"FindImage\": { \
\"results\": { \
\"list\": [\"name\"] \
}, \
\"constraints\": { \
\"name\": [ \"==\", \"ImageTransactionFailureRollback_1\" ] \
} \
} \
}, \
{ \
\"FindImage\": { \
\"results\": { \
\"list\": [\"name\"] \
}, \
\"constraints\": { \
\"name\": [ \"==\", \"ImageTransactionFailureRollback_2\" ] \
} \
} \
} \
";
string_query_image_lookup += "]";

proto_query.clear_blobs();
proto_query.set_json(string_query_image_lookup);

query_handler.process_query(proto_query, response);
json_reader.parse(response.json(), json_response);

EXPECT_EQ(json_response[0]["FindImage"]["status"].asString(), "0");
EXPECT_EQ(json_response[0]["FindImage"]["info"], "No entities found");
EXPECT_EQ(json_response[1]["FindImage"]["status"].asString(), "0");
EXPECT_EQ(json_response[1]["FindImage"]["info"], "No entities found");

// Make sure number of objects in S3 is still the same
std::unique_ptr<FILE, decltype(&pclose)> pipe2(popen(s3_num_objects_cmd, "r"), pclose);
if (!pipe2) {
throw std::runtime_error("popen() failed!");
}
while (fgets(buffer.data(), static_cast<int>(buffer.size()), pipe2.get()) != nullptr) {
result += buffer.data();
}

EXPECT_EQ(s3_num_objects, stoi(result));

} catch (...) {
printErrorMessage("ImageTransactionRollback");
}
}