diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2022-12-13 15:47:32 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-12-13 15:47:32 +0100 |
commit | 90eb4f3c8e173d275f65196d378eedbbeb87af5f (patch) | |
tree | 5683368fa0848dd2bd3193a308465aa03b9481a7 /searchcore | |
parent | fa8ba67ef82691df5f74dc2d02430931a2634ccc (diff) | |
parent | f6dca7c9b77a1f0495630bce095b229ccdf11ab9 (diff) |
Merge pull request #25237 from vespa-engine/balder/gc-unused-executor-magic
There is only one the master executor available to the MaintenanceJobs.
Diffstat (limited to 'searchcore')
5 files changed, 14 insertions, 55 deletions
diff --git a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp index 47555288a10..debad7eaf26 100644 --- a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp +++ b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp @@ -917,7 +917,7 @@ TEST_F("require that a simple maintenance job is executed", MaintenanceControlle { auto job = std::make_unique<MySimpleJob>(200ms, 200ms, 3); MySimpleJob &myJob = *job; - f._mc.registerJobInMasterThread(std::move(job)); + f._mc.registerJob(std::move(job)); f._injectDefaultJobs = false; f.startMaintenance(); bool done = myJob._latch.await(TIMEOUT); @@ -929,7 +929,7 @@ TEST_F("require that a split maintenance job is executed", MaintenanceController { auto job = std::make_unique<MySplitJob>(200ms, TIMEOUT * 2, 3); MySplitJob &myJob = *job; - f._mc.registerJobInMasterThread(std::move(job)); + f._mc.registerJob(std::move(job)); f._injectDefaultJobs = false; f.startMaintenance(); bool done = myJob._latch.await(TIMEOUT); @@ -942,7 +942,7 @@ TEST_F("require that blocked jobs are not executed", MaintenanceControllerFixtur auto job = std::make_unique<MySimpleJob>(200ms, 200ms, 0); MySimpleJob &myJob = *job; myJob.block(); - f._mc.registerJobInMasterThread(std::move(job)); + f._mc.registerJob(std::move(job)); f._injectDefaultJobs = false; f.startMaintenance(); std::this_thread::sleep_for(2s); @@ -955,8 +955,8 @@ TEST_F("require that maintenance controller state list jobs", MaintenanceControl auto job1 = std::make_unique<MySimpleJob>(TIMEOUT * 2, TIMEOUT * 2, 0); auto job2 = std::make_unique<MyLongRunningJob>(200ms, 200ms); auto &longRunningJob = dynamic_cast<MyLongRunningJob &>(*job2); - f._mc.registerJobInMasterThread(std::move(job1)); - f._mc.registerJobInMasterThread(std::move(job2)); + f._mc.registerJob(std::move(job1)); + f._mc.registerJob(std::move(job2)); f._injectDefaultJobs = false; f.startMaintenance(); longRunningJob._firstRun.await(TIMEOUT); @@ -994,14 +994,6 @@ containsJob(const MaintenanceController::JobList &jobs, const vespalib::string & return findJob(jobs, jobName) != nullptr; } -bool -containsJobAndExecutedBy(const MaintenanceController::JobList &jobs, const vespalib::string &jobName, - const vespalib::Executor & executor) -{ - const auto *job = findJob(jobs, jobName); - return (job != nullptr) && (&job->getExecutor() == &executor); -} - TEST_F("require that lid space compaction jobs can be disabled", MaintenanceControllerFixture) { f.forwardMaintenanceConfig(); @@ -1018,17 +1010,6 @@ TEST_F("require that lid space compaction jobs can be disabled", MaintenanceCont } } -TEST_F("require that maintenance jobs are run by correct executor", MaintenanceControllerFixture) -{ - f.injectMaintenanceJobs(); - auto jobs = f._mc.getJobList(); - EXPECT_EQUAL(7u, jobs.size()); - EXPECT_TRUE(containsJobAndExecutedBy(jobs, "heart_beat", f._threadService)); - EXPECT_TRUE(containsJobAndExecutedBy(jobs, "prune_removed_documents.searchdocument", f._threadService)); - EXPECT_TRUE(containsJobAndExecutedBy(jobs, "move_buckets.searchdocument", f._threadService)); - EXPECT_TRUE(containsJobAndExecutedBy(jobs, "sample_attribute_usage.searchdocument", f._threadService)); -} - void assertPruneRemovedDocumentsConfig(vespalib::duration expDelay, vespalib::duration expInterval, vespalib::duration interval, MaintenanceControllerFixture &f) { diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp b/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp index c188bcc432d..4e2dd1ea86f 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp @@ -38,7 +38,7 @@ injectLidSpaceCompactionJobs(MaintenanceController &controller, std::move(lidHandler), opStorer, controller.masterThread(), bucketExecutor, diskMemUsageNotifier,config.getBlockableJobConfig(), clusterStateChangedNotifier, calc && calc->nodeRetired(), bucketSpace); - controller.registerJobInMasterThread(trackJob(tracker, std::move(job))); + controller.registerJob(trackJob(tracker, std::move(job))); } } @@ -61,7 +61,7 @@ injectBucketMoveJob(MaintenanceController &controller, bucketExecutor, controller.getReadySubDB(), controller.getNotReadySubDB(), bucketCreateNotifier, clusterStateChangedNotifier, bucketStateChangedNotifier, diskMemUsageNotifier, config.getBlockableJobConfig(), docTypeName, bucketSpace); - controller.registerJobInMasterThread(trackJob(jobTrackers.getBucketMove(), std::move(bmj))); + controller.registerJob(trackJob(jobTrackers.getBucketMove(), std::move(bmj))); } } @@ -86,12 +86,12 @@ MaintenanceJobsInjector::injectJobs(MaintenanceController &controller, IAttributeManagerSP notReadyAttributeManager, AttributeUsageFilter &attributeUsageFilter) { - controller.registerJobInMasterThread(std::make_unique<HeartBeatJob>(hbHandler, config.getHeartBeatConfig())); + controller.registerJob(std::make_unique<HeartBeatJob>(hbHandler, config.getHeartBeatConfig())); const auto & docTypeName = controller.getDocTypeName().getName(); const MaintenanceDocumentSubDB &mRemSubDB(controller.getRemSubDB()); - controller.registerJobInMasterThread( + controller.registerJob( trackJob(jobTrackers.getRemovedDocumentsPrune(), PruneRemovedDocumentsJob::create(config.getPruneRemovedDocumentsConfig(), controller.retainDB(), *mRemSubDB.meta_store(), mRemSubDB.sub_db_id(), bucketSpace, @@ -113,7 +113,7 @@ MaintenanceJobsInjector::injectJobs(MaintenanceController &controller, moveHandler, bucketModifiedHandler, clusterStateChangedNotifier, bucketStateChangedNotifier, calc, jobTrackers, diskMemUsageNotifier); - controller.registerJobInMasterThread( + controller.registerJob( std::make_unique<SampleAttributeUsageJob>(std::move(readyAttributeManager), std::move(notReadyAttributeManager), attributeUsageFilter, docTypeName, diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.cpp b/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.cpp index adafa4c6217..c86019005dc 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.cpp @@ -30,13 +30,6 @@ public: void run() override { _job->run(); } }; -bool -isRunnable(const MaintenanceJobRunner & job, const Executor * master) { - return (&job.getExecutor() == master) - ? false - : job.isRunnable(); -} - } MaintenanceController::MaintenanceController(FNET_Transport & transport, @@ -61,18 +54,11 @@ MaintenanceController::~MaintenanceController() } void -MaintenanceController::registerJobInMasterThread(IMaintenanceJob::UP job) -{ - // Called by master write thread - registerJob(_masterThread, std::move(job)); -} - -void -MaintenanceController::registerJob(Executor & executor, IMaintenanceJob::UP job) +MaintenanceController::registerJob(IMaintenanceJob::UP job) { // Called by master write thread Guard guard(_jobsLock); - _jobs.push_back(std::make_shared<MaintenanceJobRunner>(executor, std::move(job))); + _jobs.push_back(std::make_shared<MaintenanceJobRunner>(_masterThread, std::move(job))); } void @@ -89,11 +75,6 @@ MaintenanceController::killJobs() for (auto &job : _jobs) { job->stop(); // Make sure no more tasks are added to the executor } - for (auto &job : _jobs) { - while (isRunnable(*job, &_masterThread)) { - std::this_thread::sleep_for(1ms); - } - } JobList tmpJobs; { Guard guard(_jobsLock); @@ -162,14 +143,13 @@ MaintenanceController::restart() if (!getStarted() || getStopping() || !_readySubDB.valid()) { return; } - _periodicTaskHandles.clear(); - addJobsToPeriodicTimer(); } void MaintenanceController::addJobsToPeriodicTimer() { + _periodicTaskHandles.clear(); // No need to take _jobsLock as modification of _jobs also happens in master write thread. for (const auto &jw : _jobs) { const IMaintenanceJob &job = jw->getJob(); diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.h b/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.h index af2f7e8a0fc..08e9d9c1e80 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.h +++ b/searchcore/src/vespa/searchcore/proton/server/maintenancecontroller.h @@ -45,7 +45,7 @@ public: vespalib::MonitoredRefCount& refCount, const DocTypeName& docTypeName); ~MaintenanceController(); - void registerJobInMasterThread(IMaintenanceJob::UP job); + void registerJob(IMaintenanceJob::UP job); void killJobs(); @@ -96,7 +96,6 @@ private: void addJobsToPeriodicTimer(); void restart(); void performHoldJobs(JobList jobs); - void registerJob(vespalib::Executor & executor, IMaintenanceJob::UP job); }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenancejobrunner.h b/searchcore/src/vespa/searchcore/proton/server/maintenancejobrunner.h index 17f244e6621..200f0d05de0 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenancejobrunner.h +++ b/searchcore/src/vespa/searchcore/proton/server/maintenancejobrunner.h @@ -32,7 +32,6 @@ public: void run() override; void stop(); bool isRunnable() const; - const vespalib::Executor & getExecutor() const { return _executor; } const IMaintenanceJob &getJob() const { return *_job; } IMaintenanceJob &getJob() { return *_job; } }; |