Skip to content

Commit

Permalink
refactor: sim: reuploading problem + fix memory leak in mysql.hh
Browse files Browse the repository at this point in the history
  • Loading branch information
varqox committed May 13, 2024
1 parent ec584ad commit 17a649e
Show file tree
Hide file tree
Showing 43 changed files with 1,120 additions and 1,547 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@

#include <cstdint>
#include <optional>
#include <sim/internal_files/internal_file.hh>
#include <sim/jobs/job.hh>
#include <sim/problems/problem.hh>

namespace sim::add_problem_jobs {

struct AddProblemJob {
decltype(jobs::Job::id) id;
uint64_t file_id;
decltype(internal_files::InternalFile::id) file_id;
decltype(problems::Problem::type) visibility;
bool force_time_limits_reset;
bool ignore_simfile;
Expand Down
10 changes: 10 additions & 0 deletions subprojects/sim/include/sim/internal_files/internal_file.hh
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
#pragma once

#include <cstdint>
#include <sim/mysql/mysql.hh>
#include <sim/sql/fields/datetime.hh>
#include <sim/sql/sql.hh>
#include <simlib/concat_tostr.hh>
#include <simlib/time.hh>
#include <string>

namespace sim::internal_files {
Expand All @@ -16,4 +19,11 @@ inline std::string path_of(decltype(InternalFile::id) id) {
return concat_tostr("internal_files/", id);
}

inline decltype(InternalFile::id)
new_internal_file_id(mysql::Connection& mysql, const std::string& curr_datetime = mysql_date()) {
auto stmt =
mysql.execute(sql::InsertInto("internal_files (created_at)").values("?", curr_datetime));
return stmt.insert_id();
}

} // namespace sim::internal_files
5 changes: 0 additions & 5 deletions subprojects/sim/include/sim/jobs/job.hh
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ struct Job {
(JUDGE_SUBMISSION, 1, "judge_submission")
(ADD_PROBLEM, 2, "add_problem")
(REUPLOAD_PROBLEM, 3, "reupload_problem")
(REUPLOAD_PROBLEM__JUDGE_MODEL_SOLUTION, 5, "reupload_problem__judge_model_solution")
(EDIT_PROBLEM, 6, "edit_problem")
(DELETE_PROBLEM, 7, "delete_problem")
(RESELECT_FINAL_SUBMISSIONS_IN_CONTEST_PROBLEM, 8,
Expand Down Expand Up @@ -45,7 +44,6 @@ struct Job {
uint64_t id;
sql::fields::Datetime created_at;
std::optional<decltype(internal_files::InternalFile::id)> file_id;
std::optional<decltype(internal_files::InternalFile::id)> tmp_file_id;
std::optional<decltype(users::User::id)> creator;
Type type;
uint8_t priority;
Expand All @@ -71,7 +69,6 @@ constexpr decltype(Job::priority) default_priority(Job::Type type) {
case Job::Type::EDIT_PROBLEM:
case Job::Type::CHANGE_PROBLEM_STATEMENT: return 25;
case Job::Type::RESET_PROBLEM_TIME_LIMITS_USING_MODEL_SOLUTION: return 20;
case Job::Type::REUPLOAD_PROBLEM__JUDGE_MODEL_SOLUTION: return 15;
case Job::Type::ADD_PROBLEM:
case Job::Type::REUPLOAD_PROBLEM: return 10;
case Job::Type::JUDGE_SUBMISSION: return 5;
Expand All @@ -85,7 +82,6 @@ constexpr bool is_problem_management_job(Job::Type type) {
switch (type) {
case Job::Type::ADD_PROBLEM:
case Job::Type::REUPLOAD_PROBLEM:
case Job::Type::REUPLOAD_PROBLEM__JUDGE_MODEL_SOLUTION:
case Job::Type::EDIT_PROBLEM:
case Job::Type::DELETE_PROBLEM:
case Job::Type::RESET_PROBLEM_TIME_LIMITS_USING_MODEL_SOLUTION:
Expand All @@ -111,7 +107,6 @@ constexpr bool is_submission_job(Job::Type type) {
case Job::Type::REJUDGE_SUBMISSION: return true;
case Job::Type::ADD_PROBLEM:
case Job::Type::REUPLOAD_PROBLEM:
case Job::Type::REUPLOAD_PROBLEM__JUDGE_MODEL_SOLUTION:
case Job::Type::EDIT_PROBLEM:
case Job::Type::DELETE_PROBLEM:
case Job::Type::RESELECT_FINAL_SUBMISSIONS_IN_CONTEST_PROBLEM:
Expand Down
8 changes: 0 additions & 8 deletions subprojects/sim/include/sim/jobs/old_job.hh
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ struct OldJob {
(JUDGE_SUBMISSION, 1, "judge_submission")
(ADD_PROBLEM, 2, "add_problem")
(REUPLOAD_PROBLEM, 3, "reupload_problem")
(REUPLOAD_PROBLEM__JUDGE_MODEL_SOLUTION, 5,
"reupload_problem__judge_model_solution")
(EDIT_PROBLEM, 6, "edit_problem")
(DELETE_PROBLEM, 7, "delete_problem")
(RESELECT_FINAL_SUBMISSIONS_IN_CONTEST_PROBLEM, 8,
Expand Down Expand Up @@ -51,7 +49,6 @@ struct OldJob {
uint64_t id;
old_sql_fields::Datetime created_at;
std::optional<decltype(internal_files::OldInternalFile::id)> file_id;
std::optional<decltype(internal_files::OldInternalFile::id)> tmp_file_id;
std::optional<decltype(users::User::id)> creator;
EnumVal<Type> type;
uint8_t priority;
Expand Down Expand Up @@ -80,7 +77,6 @@ constexpr decltype(OldJob::priority) default_priority(OldJob::Type type) {
case OldJob::Type::EDIT_PROBLEM:
case OldJob::Type::CHANGE_PROBLEM_STATEMENT: return 25;
case OldJob::Type::RESET_PROBLEM_TIME_LIMITS_USING_MODEL_SOLUTION: return 20;
case OldJob::Type::REUPLOAD_PROBLEM__JUDGE_MODEL_SOLUTION: return 15;
case OldJob::Type::ADD_PROBLEM:
case OldJob::Type::REUPLOAD_PROBLEM: return 10;
case OldJob::Type::JUDGE_SUBMISSION: return 5;
Expand All @@ -95,8 +91,6 @@ constexpr const char* to_string(OldJob::Type x) {
case JT::JUDGE_SUBMISSION: return "JUDGE_SUBMISSION";
case JT::ADD_PROBLEM: return "ADD_PROBLEM";
case JT::REUPLOAD_PROBLEM: return "REUPLOAD_PROBLEM";
case JT::REUPLOAD_PROBLEM__JUDGE_MODEL_SOLUTION:
return "REUPLOAD_PROBLEM__JUDGE_MODEL_SOLUTION";
case JT::EDIT_PROBLEM: return "EDIT_PROBLEM";
case JT::DELETE_PROBLEM: return "DELETE_PROBLEM";
case JT::RESELECT_FINAL_SUBMISSIONS_IN_CONTEST_PROBLEM:
Expand All @@ -121,7 +115,6 @@ constexpr bool is_problem_management_job(OldJob::Type x) {
switch (x) {
case JT::ADD_PROBLEM:
case JT::REUPLOAD_PROBLEM:
case JT::REUPLOAD_PROBLEM__JUDGE_MODEL_SOLUTION:
case JT::EDIT_PROBLEM:
case JT::DELETE_PROBLEM:
case JT::RESET_PROBLEM_TIME_LIMITS_USING_MODEL_SOLUTION:
Expand All @@ -147,7 +140,6 @@ constexpr bool is_submission_job(OldJob::Type x) {
case JT::REJUDGE_SUBMISSION: return true;
case JT::ADD_PROBLEM:
case JT::REUPLOAD_PROBLEM:
case JT::REUPLOAD_PROBLEM__JUDGE_MODEL_SOLUTION:
case JT::EDIT_PROBLEM:
case JT::DELETE_PROBLEM:
case JT::RESELECT_FINAL_SUBMISSIONS_IN_CONTEST_PROBLEM:
Expand Down
8 changes: 0 additions & 8 deletions subprojects/sim/include/sim/jobs/utils.hh
Original file line number Diff line number Diff line change
Expand Up @@ -230,14 +230,6 @@ struct MergeUsersInfo {
}
};

void restart_job(
mysql::Connection& mysql,
StringView job_id,
OldJob::Type job_type,
StringView job_info,
bool notify_job_server
);

void restart_job(mysql::Connection& mysql, StringView job_id, bool notify_job_server);

// Notifies the Job server that there are jobs to do
Expand Down
53 changes: 52 additions & 1 deletion subprojects/sim/include/sim/mysql/mysql.hh
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,58 @@ public:
~Statement() noexcept(false);

Statement(Statement&& other) noexcept;
Statement& operator=(Statement&& other) noexcept;

// There is a problem with libmariadb preventing implementation of this operator because of
// getting "Commands out of sync; you can't run this command now" error. Consider following
// code illustrating the problem:
//
// auto* stmt1 = mysql_stmt_init(conn);
// throw_assert(stmt1);
// throw_assert(!mysql_stmt_prepare(
// stmt1,
// "select 1 from users where id=1",
// strlen("select 1 from users where id=1")
// ));
// throw_assert(!mysql_stmt_execute(stmt1));
//
// int x1;
// MYSQL_BIND binds1[1];
// binds1[0].buffer_type = MYSQL_TYPE_LONG;
// binds1[0].buffer = &x1;
// binds1[0].is_unsigned = false;
// binds1[0].error = &binds1[0].error_value;
// binds1[0].is_null = &binds1[0].is_null_value;
// throw_assert(!mysql_stmt_bind_result(stmt1, binds1));
// throw_assert(!mysql_stmt_store_result(stmt1));
//
// auto stmt2 = mysql_stmt_init(conn);
// throw_assert(stmt2);
// if (mysql_stmt_prepare(
// stmt2,
// "select 1 from users where id=1",
// strlen("select 1 from users where id=1")
// )) {
// THROW(mysql_stmt_error(stmt2));
// }
// throw_assert(!mysql_stmt_execute(stmt2));
// // This is the root cause of the problem, this call has to be before
// // mysql_stmt_execute(stmt2) for the problem to disappear.
// throw_assert(!mysql_stmt_close(stmt1));
//
// int x2;
// MYSQL_BIND binds2[1];
// binds2[0].buffer_type = MYSQL_TYPE_LONG;
// binds2[0].buffer = &x2;
// binds2[0].is_unsigned = false;
// binds2[0].error = &binds2[0].error_value;
// binds2[0].is_null = &binds2[0].is_null_value;
// throw_assert(!mysql_stmt_bind_result(stmt2, binds2));
//
// if (mysql_stmt_store_result(stmt2)) {
// THROW(mysql_stmt_error(stmt2)); // Here exception "Commands out of sync;" is thrown
// }
// throw_assert(!mysql_stmt_close(stmt2));
Statement& operator=(Statement&& other) = delete;

Statement(const Statement&) = delete;
Statement& operator=(const Statement&) = delete;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#pragma once

#include <cstdint>
#include <optional>
#include <sim/jobs/job.hh>
#include <sim/problems/problem.hh>

namespace sim::reupload_problem_jobs {

struct ReuploadProblemJob {
decltype(jobs::Job::id) id;
decltype(problems::Problem::id) problem_id;
decltype(internal_files::InternalFile::id) file_id;
bool force_time_limits_reset;
bool ignore_simfile;
decltype(problems::Problem::name) name;
decltype(problems::Problem::label) label;
std::optional<uint64_t> memory_limit_in_mib;
std::optional<uint64_t> fixed_time_limit_in_ns;
bool reset_scoring;
bool look_for_new_tests;
};

} // namespace sim::reupload_problem_jobs
3 changes: 1 addition & 2 deletions subprojects/sim/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,7 @@ job_server = executable('job-server',
implicit_include_directories : false,
sources : [
'src/job_server/dispatcher.cc',
'src/job_server/job_handlers/add_or_reupload_problem__judge_main_solution_base.cc',
'src/job_server/job_handlers/add_or_reupload_problem_base.cc',
'src/job_server/job_handlers/add_or_reupload_problem/add_or_reupload_problem.cc',
'src/job_server/job_handlers/add_problem.cc',
'src/job_server/job_handlers/change_problem_statement.cc',
'src/job_server/job_handlers/delete_contest.cc',
Expand Down
23 changes: 1 addition & 22 deletions subprojects/sim/src/backup.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,27 +77,6 @@ int main2(int argc, char** argv) {

auto transaction = mysql.start_repeatable_read_transaction();
auto old_mysql = old_mysql::ConnectionView{mysql};
auto stmt = old_mysql.prepare("SELECT tmp_file_id FROM jobs "
"WHERE tmp_file_id IS NOT NULL AND status IN (?,?,?)");
stmt.bind_and_execute(
EnumVal(OldJob::Status::DONE),
EnumVal(OldJob::Status::FAILED),
EnumVal(OldJob::Status::CANCELED)
);
uint64_t tmp_file_id = 0;
stmt.res_bind_all(tmp_file_id);

auto deleter = old_mysql.prepare("DELETE FROM internal_files WHERE id=?");
// Remove jobs temporary internal files
while (stmt.next()) {
auto file_path = sim::internal_files::old_path_of(tmp_file_id);
if (access(file_path, F_OK) == 0 and
system_clock::now() - get_modification_time(file_path) > 2h)
{
deleter.bind_and_execute(tmp_file_id);
(void)unlink(file_path);
}
}

// Remove internal files that do not have an entry in internal_files
sim::PackageContents fc;
Expand All @@ -107,7 +86,7 @@ int main2(int argc, char** argv) {
orphaned_files.emplace(file.to_string());
});

stmt = old_mysql.prepare("SELECT id FROM internal_files");
auto stmt = old_mysql.prepare("SELECT id FROM internal_files");
stmt.bind_and_execute();
InplaceBuff<32> file_id;
stmt.res_bind_all(file_id);
Expand Down
31 changes: 2 additions & 29 deletions subprojects/sim/src/job_server/dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "job_handlers/reselect_final_submissions_in_contest_problem.hh"
#include "job_handlers/reset_problem_time_limits.hh"
#include "job_handlers/reupload_problem.hh"
#include "job_handlers/reupload_problem__judge_main_solution.hh"

#include <memory>
#include <sim/jobs/old_job.hh>
Expand All @@ -28,8 +27,6 @@ void job_dispatcher(
uint64_t job_id,
OldJob::Type jtype,
std::optional<uint64_t> file_id,
std::optional<uint64_t> tmp_file_id,
std::optional<StringView> creator,
std::optional<uint64_t> aux_id,
StringView info,
StringView created_at
Expand All @@ -44,11 +41,7 @@ void job_dispatcher(
switch (jtype) {
case JT::ADD_PROBLEM: job_handler = make_unique<AddProblem>(job_id); break;

case JT::REUPLOAD_PROBLEM:
job_handler = make_unique<ReuploadProblem>(
mysql, job_id, creator.value(), info, file_id.value(), tmp_file_id, aux_id.value()
);
break;
case JT::REUPLOAD_PROBLEM: job_handler = make_unique<ReuploadProblem>(job_id); break;

case JT::EDIT_PROBLEM: {
// TODO: implement it (editing Simfile for now)
Expand Down Expand Up @@ -108,12 +101,6 @@ void job_dispatcher(
job_handler = make_unique<JudgeOrRejudge>(job_id, aux_id.value(), created_at);
break;

case JT::REUPLOAD_PROBLEM__JUDGE_MODEL_SOLUTION:
job_handler = make_unique<ReuploadProblemJudgeModelSolution>(
mysql, job_id, creator.value(), info, file_id.value(), tmp_file_id, aux_id.value()
);
break;

case JT::RESET_PROBLEM_TIME_LIMITS_USING_MODEL_SOLUTION:
job_handler = make_unique<ResetProblemTimeLimits>(job_id, aux_id.value());
break;
Expand All @@ -132,22 +119,8 @@ void job_dispatcher(
auto transaction = mysql.start_repeatable_read_transaction();
auto old_mysql = old_mysql::ConnectionView{mysql};

// Add job to delete temporary file
auto stmt = old_mysql.prepare("INSERT INTO jobs(file_id, creator, type,"
" priority, status, created_at, aux_id, info, data) "
"SELECT tmp_file_id, NULL, ?, ?, ?, ?, NULL, '', '' FROM jobs"
" WHERE id=? AND tmp_file_id IS NOT NULL");
stmt.bind_and_execute(
EnumVal(OldJob::Type::DELETE_FILE),
default_priority(OldJob::Type::DELETE_FILE),
EnumVal(OldJob::Status::PENDING),
mysql_date(),
job_id
);

// Fail job
stmt = old_mysql.prepare("UPDATE jobs SET tmp_file_id=NULL, status=?, data=? "
"WHERE id=?");
auto stmt = old_mysql.prepare("UPDATE jobs SET status=?, data=? WHERE id=?");
if (job_handler) {
stmt.bind_and_execute(
EnumVal(OldJob::Status::FAILED),
Expand Down
2 changes: 0 additions & 2 deletions subprojects/sim/src/job_server/dispatcher.hh
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ void job_dispatcher(
uint64_t job_id,
sim::jobs::OldJob::Type jtype,
std::optional<uint64_t> file_id,
std::optional<uint64_t> tmp_file_id,
std::optional<StringView> creator,
std::optional<uint64_t> aux_id,
StringView info,
StringView created_at
Expand Down
Loading

0 comments on commit 17a649e

Please sign in to comment.