From 2efa228375ac27ef9b4abc1a60bdfc5e9335be9b Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Tue, 5 Dec 2023 09:43:29 +0000 Subject: Use shared executor for warmup and GC warmup executor. --- .../src/apps/tests/persistenceconformance_test.cpp | 2 +- .../src/tests/proton/docsummary/docsummary_test.cpp | 2 +- .../src/tests/proton/documentdb/documentdb_test.cpp | 2 +- searchcore/src/vespa/searchcore/bmcluster/bm_node.cpp | 2 +- .../searchcore/proton/metrics/content_proton_metrics.h | 8 ++++---- .../src/vespa/searchcore/proton/server/documentdb.cpp | 3 +-- .../proton/server/i_shared_threading_service.h | 5 ----- searchcore/src/vespa/searchcore/proton/server/proton.cpp | 2 -- .../proton/server/proton_thread_pools_explorer.cpp | 3 --- .../proton/server/proton_thread_pools_explorer.h | 2 -- .../proton/server/shared_threading_service.cpp | 5 ----- .../searchcore/proton/server/shared_threading_service.h | 2 -- .../proton/server/shared_threading_service_config.cpp | 16 +++------------- .../proton/server/shared_threading_service_config.h | 3 --- .../proton/test/mock_shared_threading_service.cpp | 7 ++----- .../proton/test/mock_shared_threading_service.h | 6 +----- .../vespa/searchcorespi/index/warmupindexcollection.cpp | 6 +++++- 17 files changed, 20 insertions(+), 56 deletions(-) diff --git a/searchcore/src/apps/tests/persistenceconformance_test.cpp b/searchcore/src/apps/tests/persistenceconformance_test.cpp index b0e2c63a677..6fd8508990b 100644 --- a/searchcore/src/apps/tests/persistenceconformance_test.cpp +++ b/searchcore/src/apps/tests/persistenceconformance_test.cpp @@ -215,7 +215,7 @@ DocumentDBFactory::DocumentDBFactory(const vespalib::string &baseDir, int tlsLis _queryLimiter(), _metricsWireService(), _summaryExecutor(8), - _shared_service(_summaryExecutor, _summaryExecutor), + _shared_service(_summaryExecutor), _tls(_shared_service.transport(), "tls", tlsListenPort, baseDir, _fileHeaderContext) {} DocumentDBFactory::~DocumentDBFactory() = default; diff --git a/searchcore/src/tests/proton/docsummary/docsummary_test.cpp b/searchcore/src/tests/proton/docsummary/docsummary_test.cpp index d6d25ee373b..e8ed780ed82 100644 --- a/searchcore/src/tests/proton/docsummary/docsummary_test.cpp +++ b/searchcore/src/tests/proton/docsummary/docsummary_test.cpp @@ -247,7 +247,7 @@ public: : _dmk(docTypeName), _fileHeaderContext(), _summaryExecutor(8), - _shared_service(_summaryExecutor, _summaryExecutor), + _shared_service(_summaryExecutor), _tls(_shared_service.transport(), "tmp", 9013, ".", _fileHeaderContext), _made_dir(std::filesystem::create_directory(std::filesystem::path("tmpdb"))), _queryLimiter(), diff --git a/searchcore/src/tests/proton/documentdb/documentdb_test.cpp b/searchcore/src/tests/proton/documentdb/documentdb_test.cpp index e02d6fd9634..dbe5cd409b3 100644 --- a/searchcore/src/tests/proton/documentdb/documentdb_test.cpp +++ b/searchcore/src/tests/proton/documentdb/documentdb_test.cpp @@ -129,7 +129,7 @@ Fixture::Fixture(bool file_config) _dummy(), _myDBOwner(), _summaryExecutor(8), - _shared_service(_summaryExecutor, _summaryExecutor), + _shared_service(_summaryExecutor), _hwInfo(), _db(), _fileHeaderContext(), diff --git a/searchcore/src/vespa/searchcore/bmcluster/bm_node.cpp b/searchcore/src/vespa/searchcore/bmcluster/bm_node.cpp index 808747034ac..e3315399ed9 100644 --- a/searchcore/src/vespa/searchcore/bmcluster/bm_node.cpp +++ b/searchcore/src/vespa/searchcore/bmcluster/bm_node.cpp @@ -497,7 +497,7 @@ MyBmNode::MyBmNode(const vespalib::string& base_dir, int base_port, uint32_t nod _metrics_wire_service(), _config_stores(), _summary_executor(8), - _shared_service(_summary_executor, _summary_executor), + _shared_service(_summary_executor), _tls(_shared_service.transport(), "tls", _tls_listen_port, _base_dir, _file_header_context), _document_db_owner(), _bucket_space(document::test::makeBucketSpace(_doc_type_name.getName())), diff --git a/searchcore/src/vespa/searchcore/proton/metrics/content_proton_metrics.h b/searchcore/src/vespa/searchcore/proton/metrics/content_proton_metrics.h index e5a3b41a487..876287eb835 100644 --- a/searchcore/src/vespa/searchcore/proton/metrics/content_proton_metrics.h +++ b/searchcore/src/vespa/searchcore/proton/metrics/content_proton_metrics.h @@ -26,18 +26,18 @@ struct ContentProtonMetrics : metrics::MetricSet ExecutorMetrics match; ExecutorMetrics docsum; ExecutorMetrics shared; - ExecutorMetrics warmup; + ExecutorMetrics warmup; //TODO not used anymore, remove ExecutorMetrics field_writer; - ProtonExecutorMetrics(metrics::MetricSet *parent); - ~ProtonExecutorMetrics(); + explicit ProtonExecutorMetrics(metrics::MetricSet *parent); + ~ProtonExecutorMetrics() override; }; struct SessionCacheMetrics : metrics::MetricSet { SessionManagerMetrics search; SessionManagerMetrics grouping; - SessionCacheMetrics(metrics::MetricSet *parent); + explicit SessionCacheMetrics(metrics::MetricSet *parent); ~SessionCacheMetrics() override; }; diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp index 9bbf1632fcd..eb7a664ae24 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp @@ -19,7 +19,6 @@ #include #include #include -#include #include #include #include @@ -218,7 +217,7 @@ DocumentDB::DocumentDB(const vespalib::string &baseDir, _transient_usage_provider(std::make_shared(*this)), _feedHandler(std::make_unique(_writeService, tlsSpec, docTypeName, *this, _writeFilter, *this, tlsWriterFactory)), _subDBs(*this, *this, *_feedHandler, _docTypeName, - _writeService, shared_service.warmup(), fileHeaderContext, std::move(attribute_interlock), + _writeService, shared_service.shared(), fileHeaderContext, std::move(attribute_interlock), metricsWireService, getMetrics(), queryLimiter, shared_service.clock(), _configMutex, _baseDir, hwInfo), _maintenanceController(shared_service.transport(), _writeService.master(), _refCount, _docTypeName), diff --git a/searchcore/src/vespa/searchcore/proton/server/i_shared_threading_service.h b/searchcore/src/vespa/searchcore/proton/server/i_shared_threading_service.h index 67eba7bb966..c09e7d51afc 100644 --- a/searchcore/src/vespa/searchcore/proton/server/i_shared_threading_service.h +++ b/searchcore/src/vespa/searchcore/proton/server/i_shared_threading_service.h @@ -21,11 +21,6 @@ class ISharedThreadingService { public: virtual ~ISharedThreadingService() = default; - /** - * Returns the executor used for warmup (e.g. index warmup). - */ - virtual vespalib::ThreadExecutor& warmup() = 0; - /** * Returns the shared executor used for various assisting tasks in a document db. * diff --git a/searchcore/src/vespa/searchcore/proton/server/proton.cpp b/searchcore/src/vespa/searchcore/proton/server/proton.cpp index 0839117f6ef..95cda167d5e 100644 --- a/searchcore/src/vespa/searchcore/proton/server/proton.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/proton.cpp @@ -852,7 +852,6 @@ Proton::updateMetrics(const metrics::MetricLockGuard &) } if (_shared_service) { metrics.shared.update(_shared_service->shared().getStats()); - metrics.warmup.update(_shared_service->warmup().getStats()); metrics.field_writer.update(_shared_service->field_writer().getStats()); } } @@ -1013,7 +1012,6 @@ Proton::get_child(vespalib::stringref name) const (_summaryEngine) ? &_summaryEngine->get_executor() : nullptr, (_flushEngine) ? &_flushEngine->get_executor() : nullptr, &_executor, - (_shared_service) ? &_shared_service->warmup() : nullptr, (_shared_service) ? &_shared_service->field_writer() : nullptr); } else if (name == HW_INFO) { diff --git a/searchcore/src/vespa/searchcore/proton/server/proton_thread_pools_explorer.cpp b/searchcore/src/vespa/searchcore/proton/server/proton_thread_pools_explorer.cpp index ee0e41046fb..d18b0f14a7f 100644 --- a/searchcore/src/vespa/searchcore/proton/server/proton_thread_pools_explorer.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/proton_thread_pools_explorer.cpp @@ -17,14 +17,12 @@ ProtonThreadPoolsExplorer::ProtonThreadPoolsExplorer(const ThreadExecutor* share const ThreadExecutor* docsum, const ThreadExecutor* flush, const ThreadExecutor* proton, - const ThreadExecutor* warmup, vespalib::ISequencedTaskExecutor* field_writer) : _shared(shared), _match(match), _docsum(docsum), _flush(flush), _proton(proton), - _warmup(warmup), _field_writer(field_writer) { } @@ -39,7 +37,6 @@ ProtonThreadPoolsExplorer::get_state(const vespalib::slime::Inserter& inserter, convert_executor_to_slime(_docsum, object.setObject("docsum")); convert_executor_to_slime(_flush, object.setObject("flush")); convert_executor_to_slime(_proton, object.setObject("proton")); - convert_executor_to_slime(_warmup, object.setObject("warmup")); } } diff --git a/searchcore/src/vespa/searchcore/proton/server/proton_thread_pools_explorer.h b/searchcore/src/vespa/searchcore/proton/server/proton_thread_pools_explorer.h index b9ca31edd44..c2356b7dfed 100644 --- a/searchcore/src/vespa/searchcore/proton/server/proton_thread_pools_explorer.h +++ b/searchcore/src/vespa/searchcore/proton/server/proton_thread_pools_explorer.h @@ -21,7 +21,6 @@ private: const vespalib::ThreadExecutor* _docsum; const vespalib::ThreadExecutor* _flush; const vespalib::ThreadExecutor* _proton; - const vespalib::ThreadExecutor* _warmup; vespalib::ISequencedTaskExecutor* _field_writer; public: @@ -30,7 +29,6 @@ public: const vespalib::ThreadExecutor* docsum, const vespalib::ThreadExecutor* flush, const vespalib::ThreadExecutor* proton, - const vespalib::ThreadExecutor* warmup, vespalib::ISequencedTaskExecutor* field_writer); void get_state(const vespalib::slime::Inserter& inserter, bool full) const override; diff --git a/searchcore/src/vespa/searchcore/proton/server/shared_threading_service.cpp b/searchcore/src/vespa/searchcore/proton/server/shared_threading_service.cpp index 26c296fdb90..45b5ec2ef48 100644 --- a/searchcore/src/vespa/searchcore/proton/server/shared_threading_service.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/shared_threading_service.cpp @@ -2,7 +2,6 @@ #include "shared_threading_service.h" #include -#include #include #include #include @@ -21,9 +20,6 @@ SharedThreadingService::SharedThreadingService(const SharedThreadingServiceConfi FNET_Transport& transport, storage::spi::BucketExecutor& bucket_executor) : _transport(transport), - _warmup(std::make_unique(cfg.warmup_threads(), - CpuUsage::wrap(proton_warmup_executor, CpuUsage::Category::COMPACT), - cfg.shared_task_limit())), _shared(std::make_shared(cfg.shared_threads(), cfg.shared_task_limit(), vespalib::be_nice(proton_shared_executor, cfg.feeding_niceness()))), _field_writer(), @@ -51,7 +47,6 @@ SharedThreadingService::~SharedThreadingService() = default; void SharedThreadingService::sync_all_executors() { - _warmup->sync(); _shared->sync(); if (_field_writer) { _field_writer->sync_all(); diff --git a/searchcore/src/vespa/searchcore/proton/server/shared_threading_service.h b/searchcore/src/vespa/searchcore/proton/server/shared_threading_service.h index 329fba905f2..c2fffcc4a6c 100644 --- a/searchcore/src/vespa/searchcore/proton/server/shared_threading_service.h +++ b/searchcore/src/vespa/searchcore/proton/server/shared_threading_service.h @@ -20,7 +20,6 @@ class SharedThreadingService : public ISharedThreadingService { private: using Registration = std::unique_ptr; FNET_Transport & _transport; - std::unique_ptr _warmup; std::shared_ptr _shared; std::unique_ptr _field_writer; std::unique_ptr _invokeService; @@ -36,7 +35,6 @@ public: std::shared_ptr shared_raw() { return _shared; } void sync_all_executors(); - vespalib::ThreadExecutor& warmup() override { return *_warmup; } vespalib::ThreadExecutor& shared() override { return *_shared; } vespalib::ISequencedTaskExecutor& field_writer() override { return *_field_writer; } vespalib::InvokeService & invokeService() override { return *_invokeService; } diff --git a/searchcore/src/vespa/searchcore/proton/server/shared_threading_service_config.cpp b/searchcore/src/vespa/searchcore/proton/server/shared_threading_service_config.cpp index 34fc8a99b1a..0e198f7a986 100644 --- a/searchcore/src/vespa/searchcore/proton/server/shared_threading_service_config.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/shared_threading_service_config.cpp @@ -10,13 +10,11 @@ using ProtonConfig = SharedThreadingServiceConfig::ProtonConfig; SharedThreadingServiceConfig::SharedThreadingServiceConfig(uint32_t shared_threads_in, uint32_t shared_task_limit_in, - uint32_t warmup_threads_in, uint32_t field_writer_threads_in, double feeding_niceness_in, const ThreadingServiceConfig& field_writer_config_in) : _shared_threads(shared_threads_in), _shared_task_limit(shared_task_limit_in), - _warmup_threads(warmup_threads_in), _field_writer_threads(field_writer_threads_in), _feeding_niceness(feeding_niceness_in), _field_writer_config(field_writer_config_in) @@ -28,17 +26,12 @@ namespace { uint32_t derive_shared_threads(const ProtonConfig& cfg, const vespalib::HwInfo::Cpu& cpu_info) { - uint32_t scaled_cores = uint32_t(std::ceil(cpu_info.cores() * cfg.feeding.concurrency)); + auto scaled_cores = uint32_t(std::ceil(cpu_info.cores() * cfg.feeding.concurrency)); // We need at least 1 guaranteed free worker in order to ensure progress. return std::max(scaled_cores, uint32_t(cfg.flush.maxconcurrent + 1u)); } -uint32_t -derive_warmup_threads(const vespalib::HwInfo::Cpu& cpu_info) { - return std::max(1u, std::min(4u, cpu_info.cores()/8)); -} - uint32_t derive_field_writer_threads(const ProtonConfig& cfg, const vespalib::HwInfo::Cpu& cpu_info) { @@ -60,11 +53,8 @@ SharedThreadingServiceConfig::make(const proton::SharedThreadingServiceConfig::P { uint32_t shared_threads = derive_shared_threads(cfg, cpu_info); uint32_t field_writer_threads = derive_field_writer_threads(cfg, cpu_info); - return proton::SharedThreadingServiceConfig(shared_threads, shared_threads * 16, - derive_warmup_threads(cpu_info), - field_writer_threads, - cfg.feeding.niceness, - ThreadingServiceConfig::make(cfg)); + return {shared_threads, shared_threads * 16, field_writer_threads, + cfg.feeding.niceness, ThreadingServiceConfig::make(cfg)}; } } diff --git a/searchcore/src/vespa/searchcore/proton/server/shared_threading_service_config.h b/searchcore/src/vespa/searchcore/proton/server/shared_threading_service_config.h index 9292ba3db74..2b63529485f 100644 --- a/searchcore/src/vespa/searchcore/proton/server/shared_threading_service_config.h +++ b/searchcore/src/vespa/searchcore/proton/server/shared_threading_service_config.h @@ -18,7 +18,6 @@ public: private: uint32_t _shared_threads; uint32_t _shared_task_limit; - uint32_t _warmup_threads; uint32_t _field_writer_threads; double _feeding_niceness; ThreadingServiceConfig _field_writer_config; @@ -26,7 +25,6 @@ private: public: SharedThreadingServiceConfig(uint32_t shared_threads_in, uint32_t shared_task_limit_in, - uint32_t warmup_threads_in, uint32_t field_writer_threads_in, double feed_niceness_in, const ThreadingServiceConfig& field_writer_config_in); @@ -35,7 +33,6 @@ public: uint32_t shared_threads() const { return _shared_threads; } uint32_t shared_task_limit() const { return _shared_task_limit; } - uint32_t warmup_threads() const { return _warmup_threads; } uint32_t field_writer_threads() const { return _field_writer_threads; } double feeding_niceness() const { return _feeding_niceness; } const ThreadingServiceConfig& field_writer_config() const { return _field_writer_config; } diff --git a/searchcore/src/vespa/searchcore/proton/test/mock_shared_threading_service.cpp b/searchcore/src/vespa/searchcore/proton/test/mock_shared_threading_service.cpp index 7a17151fb77..95e87247366 100644 --- a/searchcore/src/vespa/searchcore/proton/test/mock_shared_threading_service.cpp +++ b/searchcore/src/vespa/searchcore/proton/test/mock_shared_threading_service.cpp @@ -7,11 +7,8 @@ VESPA_THREAD_STACK_TAG(mock_field_writer_executor) namespace proton { -MockSharedThreadingService::MockSharedThreadingService(ThreadExecutor& warmup_in, - ThreadExecutor& shared_in, - size_t num_bucket_executors) - : _warmup(warmup_in), - _shared(shared_in), +MockSharedThreadingService::MockSharedThreadingService(ThreadExecutor& shared_in, size_t num_bucket_executors) + : _shared(shared_in), _field_writer(vespalib::SequencedTaskExecutor::create(mock_field_writer_executor, 1)), _invokeService(10ms), _transport(), diff --git a/searchcore/src/vespa/searchcore/proton/test/mock_shared_threading_service.h b/searchcore/src/vespa/searchcore/proton/test/mock_shared_threading_service.h index b996d60f21e..b48d2e0df79 100644 --- a/searchcore/src/vespa/searchcore/proton/test/mock_shared_threading_service.h +++ b/searchcore/src/vespa/searchcore/proton/test/mock_shared_threading_service.h @@ -12,7 +12,6 @@ namespace proton { class MockSharedThreadingService : public ISharedThreadingService { private: using ThreadExecutor = vespalib::ThreadExecutor; - ThreadExecutor & _warmup; ThreadExecutor & _shared; std::unique_ptr _field_writer; vespalib::InvokeServiceImpl _invokeService; @@ -20,11 +19,8 @@ private: storage::spi::dummy::DummyBucketExecutor _bucket_executor; vespalib::Clock _clock; public: - MockSharedThreadingService(ThreadExecutor& warmup_in, - ThreadExecutor& shared_in, - size_t num_bucket_executors = 2); + MockSharedThreadingService(ThreadExecutor& shared_in, size_t num_bucket_executors = 2); ~MockSharedThreadingService() override; - ThreadExecutor& warmup() override { return _warmup; } ThreadExecutor& shared() override { return _shared; } vespalib::ISequencedTaskExecutor& field_writer() override { return *_field_writer; } vespalib::InvokeService & invokeService() override { return _invokeService; } diff --git a/searchcore/src/vespa/searchcorespi/index/warmupindexcollection.cpp b/searchcore/src/vespa/searchcorespi/index/warmupindexcollection.cpp index 5e3fe3ee0f5..46dfcda119d 100644 --- a/searchcore/src/vespa/searchcorespi/index/warmupindexcollection.cpp +++ b/searchcore/src/vespa/searchcorespi/index/warmupindexcollection.cpp @@ -163,7 +163,11 @@ WarmupIndexCollection::fireWarmup(Task::UP task) { vespalib::steady_time now(vespalib::steady_clock::now()); if (now < _warmupEndTime) { - _executor.execute(std::move(task)); + auto bounced = _executor.execute(std::move(task)); + if (bounced) { + //TODO Reduce to debug + LOG(warning, "Warmup prohibited due to overload."); + } } else { std::unique_lock guard(_lock); if (_warmupEndTime != vespalib::steady_time()) { -- cgit v1.2.3