From e8bd21b9d4cde4e6804685b213e0829a3ec44ab6 Mon Sep 17 00:00:00 2001 From: Phillip Smith Date: Mon, 13 Jul 2020 12:00:45 +1000 Subject: [PATCH 1/5] remove extra whitespace --- plugins/Jobs.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/Jobs.cpp b/plugins/Jobs.cpp index ff9bc76e3..089f4b9ef 100644 --- a/plugins/Jobs.cpp +++ b/plugins/Jobs.cpp @@ -1238,7 +1238,7 @@ void BedrockJobsCommand::process(SQLite& db) { else if (SIEquals(requestVerb, "RequeueJobs")) { SINFO("Requeueing jobs with IDs: " << request["jobIDs"]); list jobIDs = SParseIntegerList(request["jobIDs"]); - + if (jobIDs.size()) { const string& name = request["name"]; string nameQuery = name.empty() ? "" : ", name = " + SQ(name) + ""; From 95380a7ad4789b0aa240e5420f65181b5c738dfa Mon Sep 17 00:00:00 2001 From: Phillip Smith Date: Wed, 15 Jul 2020 10:26:17 +1000 Subject: [PATCH 2/5] use db.verifyIndex() at start instead of always creating missing indexes --- plugins/Jobs.cpp | 29 +++++++++++++++++++++++++---- plugins/Jobs.h | 2 ++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/plugins/Jobs.cpp b/plugins/Jobs.cpp index 089f4b9ef..78ac19786 100644 --- a/plugins/Jobs.cpp +++ b/plugins/Jobs.cpp @@ -49,7 +49,9 @@ BedrockJobsCommand::BedrockJobsCommand(SQLiteCommand&& baseCommand, BedrockPlugi { } -BedrockPlugin_Jobs::BedrockPlugin_Jobs(BedrockServer& s) : BedrockPlugin(s) +BedrockPlugin_Jobs::BedrockPlugin_Jobs(BedrockServer& s) : + BedrockPlugin(s), + isLive(server.args.isSet("-live")) { } @@ -100,9 +102,28 @@ void BedrockPlugin_Jobs::upgradeDatabase(SQLite& db) { // These indexes are not used by the Bedrock::Jobs plugin, but provided for easy analysis // using the Bedrock::DB plugin. - SASSERT(db.write("CREATE INDEX IF NOT EXISTS jobsName ON jobs ( name );")); - SASSERT(db.write("CREATE INDEX IF NOT EXISTS jobsParentJobIDState ON jobs ( parentJobID, state ) WHERE parentJobID != 0;")); - SASSERT(db.write("CREATE INDEX IF NOT EXISTS jobsStatePriorityNextRunName ON jobs ( state, priority, nextRun, name );")); + // TODO: we can't set isProductionServer by calling BedrockPlugin_Auth::isLiveServer() since this is bedrock jobs + SASSERT(db.verifyIndex( + "jobsName", // index name + "jobs", // table name + "( name )", // index criteria + false, // is unique? + false + )); + SASSERT(db.verifyIndex( + "jobsParentJobIDState", + "jobs", + "( parentJobID, state ) WHERE parentJobID != 0", + false, + !BedrockPlugin_Jobs::isLive + )); + SASSERT(db.verifyIndex( + "jobsStatePriorityNextRunName", + "jobs", + "( state, priority, nextRun, name )", + false, + !BedrockPlugin_Jobs::isLive + )); } // ========================================================================== diff --git a/plugins/Jobs.h b/plugins/Jobs.h index 507fb8c5b..38af1e386 100644 --- a/plugins/Jobs.h +++ b/plugins/Jobs.h @@ -16,6 +16,8 @@ class BedrockPlugin_Jobs : public BedrockPlugin { // Set of supported verbs for jobs with case-insensitive matching. static const setsupportedRequestVerbs; + const bool isLive; + private: static int64_t getNextID(SQLite& db); static const string name; From 7636a5ad5aa55c09483cc9da037f7785f74f7020 Mon Sep 17 00:00:00 2001 From: Phillip Smith Date: Wed, 15 Jul 2020 12:17:56 +1000 Subject: [PATCH 3/5] cleanup style; remove dev comments --- plugins/Jobs.cpp | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/plugins/Jobs.cpp b/plugins/Jobs.cpp index 78ac19786..de3b94af9 100644 --- a/plugins/Jobs.cpp +++ b/plugins/Jobs.cpp @@ -103,27 +103,9 @@ void BedrockPlugin_Jobs::upgradeDatabase(SQLite& db) { // These indexes are not used by the Bedrock::Jobs plugin, but provided for easy analysis // using the Bedrock::DB plugin. // TODO: we can't set isProductionServer by calling BedrockPlugin_Auth::isLiveServer() since this is bedrock jobs - SASSERT(db.verifyIndex( - "jobsName", // index name - "jobs", // table name - "( name )", // index criteria - false, // is unique? - false - )); - SASSERT(db.verifyIndex( - "jobsParentJobIDState", - "jobs", - "( parentJobID, state ) WHERE parentJobID != 0", - false, - !BedrockPlugin_Jobs::isLive - )); - SASSERT(db.verifyIndex( - "jobsStatePriorityNextRunName", - "jobs", - "( state, priority, nextRun, name )", - false, - !BedrockPlugin_Jobs::isLive - )); + SASSERT(db.verifyIndex("jobsName", "jobs", "( name )", false, !BedrockPlugin_Jobs::isLive)); + SASSERT(db.verifyIndex("jobsParentJobIDState", "jobs", "( parentJobID, state ) WHERE parentJobID != 0", false, !BedrockPlugin_Jobs::isLive)); + SASSERT(db.verifyIndex("jobsStatePriorityNextRunName", "jobs", "( state, priority, nextRun, name )", false, !BedrockPlugin_Jobs::isLive)); } // ========================================================================== From b7a24a0f7448e4d30412f1a0ecc7bcb9a953b260 Mon Sep 17 00:00:00 2001 From: Phillip Smith Date: Thu, 16 Jul 2020 10:14:20 +1000 Subject: [PATCH 4/5] remove TODO comment --- plugins/Jobs.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/plugins/Jobs.cpp b/plugins/Jobs.cpp index de3b94af9..fc269d9d7 100644 --- a/plugins/Jobs.cpp +++ b/plugins/Jobs.cpp @@ -100,9 +100,7 @@ void BedrockPlugin_Jobs::upgradeDatabase(SQLite& db) { "retryAfter TEXT NOT NULL DEFAULT \"\")", ignore)); - // These indexes are not used by the Bedrock::Jobs plugin, but provided for easy analysis - // using the Bedrock::DB plugin. - // TODO: we can't set isProductionServer by calling BedrockPlugin_Auth::isLiveServer() since this is bedrock jobs + // These indexes are not used by the Bedrock::Jobs plugin, but provided for easy analysis using the Bedrock::DB plugin. SASSERT(db.verifyIndex("jobsName", "jobs", "( name )", false, !BedrockPlugin_Jobs::isLive)); SASSERT(db.verifyIndex("jobsParentJobIDState", "jobs", "( parentJobID, state ) WHERE parentJobID != 0", false, !BedrockPlugin_Jobs::isLive)); SASSERT(db.verifyIndex("jobsStatePriorityNextRunName", "jobs", "( state, priority, nextRun, name )", false, !BedrockPlugin_Jobs::isLive)); From 9ee0520a79e408a037c1a219cf2a2fa451fee229 Mon Sep 17 00:00:00 2001 From: Phillip Smith Date: Fri, 17 Jul 2020 11:08:13 +1000 Subject: [PATCH 5/5] remove outdated comment --- plugins/Jobs.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/Jobs.cpp b/plugins/Jobs.cpp index fc269d9d7..f9cbfcc79 100644 --- a/plugins/Jobs.cpp +++ b/plugins/Jobs.cpp @@ -99,8 +99,7 @@ void BedrockPlugin_Jobs::upgradeDatabase(SQLite& db) { "parentJobID INTEGER NOT NULL DEFAULT 0, " "retryAfter TEXT NOT NULL DEFAULT \"\")", ignore)); - - // These indexes are not used by the Bedrock::Jobs plugin, but provided for easy analysis using the Bedrock::DB plugin. + // verify and conditionally create indexes SASSERT(db.verifyIndex("jobsName", "jobs", "( name )", false, !BedrockPlugin_Jobs::isLive)); SASSERT(db.verifyIndex("jobsParentJobIDState", "jobs", "( parentJobID, state ) WHERE parentJobID != 0", false, !BedrockPlugin_Jobs::isLive)); SASSERT(db.verifyIndex("jobsStatePriorityNextRunName", "jobs", "( state, priority, nextRun, name )", false, !BedrockPlugin_Jobs::isLive));