Merge 'auth: do not include unused headers' from Kefu Chai

these unused includes were identified by clangd. see https://clangd.llvm.org/guides/include-cleaner#unused-include-warning for more details on the "Unused include" warning.

Closes scylladb/scylladb#16868

* github.com:scylladb/scylladb:
  auth: do not include unused headers
  locator: Handle replication factor of 0 for initial_tablets calculations
  table: add_sstable_and_update_cache: trigger compaction only in compaction group
  compaction_manager: perform_task_on_all_files: return early when there are no sstables to compact
  compaction_manager: perform_cleanup: use compaction_manager::eligible_for_compaction

auth/allow_all_authorizer.hh

@@ -9,7 +9,6 @@
 #pragma once
 
 #include "auth/authorizer.hh"
-#include "exceptions/exceptions.hh"
 
 namespace cql3 {
 class query_processor;

auth/authentication_options.hh

@@ -8,7 +8,6 @@
 
 #pragma once
 
-#include <iosfwd>
 #include <optional>
 #include <stdexcept>
 #include <unordered_map>

auth/authenticator.cc

@@ -11,10 +11,6 @@
 #include "auth/authenticator.hh"
 
 #include "auth/authenticated_user.hh"
-#include "auth/common.hh"
-#include "auth/password_authenticator.hh"
-#include "cql3/query_processor.hh"
-#include "utils/class_registrator.hh"
 
 const sstring auth::authenticator::USERNAME_KEY("username");
 const sstring auth::authenticator::PASSWORD_KEY("password");

auth/authenticator.hh

@@ -12,8 +12,6 @@
 
 #include <string_view>
 #include <memory>
-#include <set>
-#include <stdexcept>
 #include <unordered_map>
 #include <optional>
 #include <functional>
@@ -26,9 +24,6 @@
 #include "auth/authentication_options.hh"
 #include "auth/resource.hh"
 #include "auth/sasl_challenge.hh"
-#include "bytes.hh"
-#include "enum_set.hh"
-#include "exceptions/exceptions.hh"
 
 namespace db {
     class config;

auth/authorizer.hh

@@ -11,8 +11,6 @@
 #pragma once
 
 #include <string_view>
-#include <functional>
-#include <optional>
 #include <stdexcept>
 #include <tuple>
 #include <vector>

auth/common.cc

@@ -6,14 +6,14 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
-#include <seastar/core/coroutine.hh>
 #include "auth/common.hh"
 
+#include <seastar/core/coroutine.hh>
 #include <seastar/core/shared_ptr.hh>
 
+#include "utils/exponential_backoff_retry.hh"
 #include "cql3/query_processor.hh"
 #include "cql3/statements/create_table_statement.hh"
-#include "replica/database.hh"
 #include "schema/schema_builder.hh"
 #include "service/migration_manager.hh"
 #include "timeout_config.hh"

auth/common.hh

@@ -8,7 +8,6 @@
 
 #pragma once
 
-#include <chrono>
 #include <string_view>
 
 #include <seastar/core/future.hh>
@@ -19,9 +18,7 @@
 #include <seastar/core/sstring.hh>
 #include <seastar/core/smp.hh>
 
-#include "log.hh"
 #include "seastarx.hh"
-#include "utils/exponential_backoff_retry.hh"
 
 using namespace std::chrono_literals;
 

auth/default_authorizer.cc

@@ -15,14 +15,11 @@ extern "C" {
 #include <unistd.h>
 }
 
-#include <chrono>
-#include <random>
-
 #include <boost/algorithm/string/join.hpp>
 #include <boost/range.hpp>
 #include <seastar/core/seastar.hh>
+#include <seastar/core/sleep.hh>
 
-#include "auth/authenticated_user.hh"
 #include "auth/common.hh"
 #include "auth/permission.hh"
 #include "auth/role_or_anonymous.hh"
@@ -30,7 +27,6 @@ extern "C" {
 #include "cql3/untyped_result_set.hh"
 #include "exceptions/exceptions.hh"
 #include "log.hh"
-#include "replica/database.hh"
 #include "utils/class_registrator.hh"
 
 namespace auth {

auth/default_authorizer.hh

@@ -10,8 +10,6 @@
 
 #pragma once
 
-#include <functional>
-
 #include <seastar/core/abort_source.hh>
 
 #include "auth/authorizer.hh"

auth/maintenance_socket_role_manager.cc

@@ -11,7 +11,6 @@
 #include <seastar/core/future.hh>
 #include <stdexcept>
 #include <string_view>
-#include "log.hh"
 #include "utils/class_registrator.hh"
 
 namespace auth {

auth/maintenance_socket_role_manager.hh

@@ -10,9 +10,7 @@
 
 #include "auth/resource.hh"
 #include "auth/role_manager.hh"
-#include "authorizer.hh"
 #include "seastar/core/future.hh"
-#include <stdexcept>
 
 namespace cql3 {
 class query_processor;

auth/password_authenticator.cc

@@ -10,14 +10,13 @@
 
 #include "auth/password_authenticator.hh"
 
-#include <algorithm>
-#include <chrono>
 #include <random>
 #include <string_view>
 #include <optional>
 
 #include <boost/algorithm/cxx11/all_of.hpp>
 #include <seastar/core/seastar.hh>
+#include <seastar/core/sleep.hh>
 
 #include "auth/authenticated_user.hh"
 #include "auth/common.hh"

auth/password_authenticator.hh

@@ -12,6 +12,7 @@
 
 #include <seastar/core/abort_source.hh>
 
+#include "db/consistency_level_type.hh"
 #include "auth/authenticator.hh"
 
 namespace db {

auth/passwords.cc

@@ -9,7 +9,6 @@
 #include "auth/passwords.hh"
 
 #include <cerrno>
-#include <optional>
 
 extern "C" {
 #include <crypt.h>

auth/permissions_cache.cc

@@ -9,7 +9,6 @@
 #include "auth/permissions_cache.hh"
 
 #include "auth/authorizer.hh"
-#include "auth/common.hh"
 #include "auth/service.hh"
 
 namespace auth {

auth/permissions_cache.hh

@@ -8,11 +8,7 @@
 
 #pragma once
 
-#include <chrono>
-#include <string_view>
-#include <functional>
 #include <iostream>
-#include <optional>
 #include <utility>
 
 #include <fmt/core.h>
@@ -20,7 +16,6 @@
 #include <seastar/core/shared_ptr.hh>
 #include <seastar/core/sstring.hh>
 
-#include "auth/authenticated_user.hh"
 #include "auth/permission.hh"
 #include "auth/resource.hh"
 #include "auth/role_or_anonymous.hh"

auth/resource.cc

@@ -19,8 +19,6 @@
 #include <boost/algorithm/string/split.hpp>
 #include <boost/algorithm/string/classification.hpp>
 
-#include "service/storage_proxy.hh"
-#include "data_dictionary/user_types_metadata.hh"
 #include "cql3/util.hh"
 #include "db/marshal/type_parser.hh"
 

auth/role_manager.hh

@@ -11,7 +11,6 @@
 #include <string_view>
 #include <memory>
 #include <optional>
-#include <stdexcept>
 #include <unordered_set>
 
 #include <seastar/core/future.hh>

auth/service.cc

@@ -11,7 +11,6 @@
 #include "auth/service.hh"
 
 #include <algorithm>
-#include <map>
 
 #include <seastar/core/future-util.hh>
 #include <seastar/core/sharded.hh>
@@ -21,20 +20,17 @@
 #include "auth/allow_all_authorizer.hh"
 #include "auth/common.hh"
 #include "auth/role_or_anonymous.hh"
-#include "cql3/functions/function_name.hh"
 #include "cql3/functions/functions.hh"
 #include "cql3/query_processor.hh"
 #include "cql3/untyped_result_set.hh"
 #include "db/config.hh"
 #include "db/consistency_level_type.hh"
 #include "db/functions/function_name.hh"
-#include "exceptions/exceptions.hh"
 #include "log.hh"
 #include "service/migration_manager.hh"
 #include "utils/class_registrator.hh"
 #include "locator/abstract_replication_strategy.hh"
 #include "data_dictionary/keyspace_metadata.hh"
-#include "mutation/mutation.hh"
 
 namespace auth {
 

auth/standard_role_manager.cc

@@ -27,7 +27,6 @@
 #include "exceptions/exceptions.hh"
 #include "log.hh"
 #include "utils/class_registrator.hh"
-#include "replica/database.hh"
 #include "service/migration_manager.hh"
 #include "password_authenticator.hh"
 

auth/standard_role_manager.hh

@@ -11,7 +11,6 @@
 #include "auth/role_manager.hh"
 
 #include <string_view>
-#include <unordered_set>
 
 #include <seastar/core/abort_source.hh>
 #include <seastar/core/future.hh>

compaction/compaction_manager.cc

@@ -1627,6 +1627,9 @@ future<compaction_manager::compaction_stats_opt> compaction_manager::perform_tas
             return a->data_size() > b->data_size();
         });
     });
+    if (sstables.empty()) {
+        co_return std::nullopt;
+    }
     co_return co_await perform_compaction<TaskType>(throw_if_stopping::no, info, &t, info.value_or(tasks::task_info{}).id, std::move(options), std::move(owned_ranges_ptr), std::move(sstables), std::move(compacting), std::forward<Args>(args)...);
 }
 
@@ -1892,7 +1895,7 @@ future<> compaction_manager::perform_cleanup(owned_ranges_ptr sorted_owned_range
 
         auto has_sstables_eligible_for_compaction = [&] {
             for (auto& sst : cs.sstables_requiring_cleanup) {
-                if (sstables::is_eligible_for_compaction(sst)) {
+                if (eligible_for_compaction(sst)) {
                     return true;
                 }
             }

locator/network_topology_strategy.cc

@@ -305,7 +305,7 @@ static unsigned calculate_initial_tablets_from_topology(const schema& s, const t
             rf_in_dc = it->second;
         }
 
-        unsigned tablets_in_dc = (shards_in_dc + rf_in_dc - 1) / rf_in_dc;
+        unsigned tablets_in_dc = rf_in_dc > 0 ? (shards_in_dc + rf_in_dc - 1) / rf_in_dc : 0;
         initial_tablets = std::max(initial_tablets, tablets_in_dc);
     }
     rslogger.debug("Estimated {} initial tablets for table {}.{}", initial_tablets, s.ks_name(), s.cf_name());

replica/database.hh

@@ -602,7 +602,7 @@ private:
         return _config.enable_cache && _schema->caching_options().enabled();
     }
     void update_stats_for_new_sstable(const sstables::shared_sstable& sst) noexcept;
-    future<> do_add_sstable_and_update_cache(sstables::shared_sstable sst, sstables::offstrategy offstrategy);
+    future<> do_add_sstable_and_update_cache(sstables::shared_sstable sst, sstables::offstrategy offstrategy, bool trigger_compaction);
     // Helpers which add sstable on behalf of a compaction group and refreshes compound set.
     void add_sstable(compaction_group& cg, sstables::shared_sstable sstable);
     void add_maintenance_sstable(compaction_group& cg, sstables::shared_sstable sst);

replica/table.cc

@@ -814,9 +814,9 @@ void table::update_stats_for_new_sstable(const sstables::shared_sstable& sst) no
 }
 
 future<>
-table::do_add_sstable_and_update_cache(sstables::shared_sstable sst, sstables::offstrategy offstrategy) {
+table::do_add_sstable_and_update_cache(sstables::shared_sstable sst, sstables::offstrategy offstrategy, bool trigger_compaction) {
     auto permit = co_await seastar::get_units(_sstable_set_mutation_sem, 1);
-    co_return co_await get_row_cache().invalidate(row_cache::external_updater([this, sst, offstrategy] () noexcept {
+    co_return co_await get_row_cache().invalidate(row_cache::external_updater([&] () noexcept {
         // FIXME: this is not really noexcept, but we need to provide strong exception guarantees.
         // atomically load all opened sstables into column family.
         compaction_group& cg = compaction_group_for_sstable(sst);
@@ -826,20 +826,24 @@ table::do_add_sstable_and_update_cache(sstables::shared_sstable sst, sstables::o
             add_maintenance_sstable(cg, sst);
         }
         update_stats_for_new_sstable(sst);
+        if (trigger_compaction) {
+            try_trigger_compaction(cg);
+        }
     }), dht::partition_range::make({sst->get_first_decorated_key(), true}, {sst->get_last_decorated_key(), true}));
 }
 
 future<>
 table::add_sstable_and_update_cache(sstables::shared_sstable sst, sstables::offstrategy offstrategy) {
-    co_await do_add_sstable_and_update_cache(std::move(sst), offstrategy);
-    trigger_compaction();
+    bool do_trigger_compaction = offstrategy == sstables::offstrategy::no;
+    co_await do_add_sstable_and_update_cache(std::move(sst), offstrategy, do_trigger_compaction);
 }
 
 future<>
 table::add_sstables_and_update_cache(const std::vector<sstables::shared_sstable>& ssts) {
+    constexpr bool do_not_trigger_compaction = false;
     for (auto& sst : ssts) {
         try {
-            co_await do_add_sstable_and_update_cache(sst, sstables::offstrategy::no);
+            co_await do_add_sstable_and_update_cache(sst, sstables::offstrategy::no, do_not_trigger_compaction);
         } catch (...) {
             tlogger.error("Failed to load SSTable {}: {}", sst->toc_filename(), std::current_exception());
             throw;

test/rest_api/task_manager_utils.py

@@ -56,7 +56,7 @@ def assert_task_does_not_exist(rest_api, task_id):
     assert resp.status_code == requests.codes.bad_request, f"Task {task_id} is kept in memory"
 
 def check_child_parent_relationship(rest_api, parent, tree_depth, allow_no_children, depth=0):
-    assert allow_no_children or parent.get("children_ids", []), "Child tasks were not created"
+    assert allow_no_children or parent.get("children_ids", []), f"Child tasks were not created for {parent}"
 
     for child_id in parent.get("children_ids", []):
         child = wait_for_task(rest_api, child_id)

test/rest_api/test_compaction_task.py

@@ -91,7 +91,7 @@ def test_offstrategy_keyspace_compaction_task(cql, this_dc, rest_api):
     check_compaction_task(cql, this_dc, rest_api, lambda keyspace, _: rest_api.send("POST", f"storage_service/keyspace_offstrategy_compaction/{keyspace}"), "offstrategy compaction", task_tree_depth, True)
 
 def test_rewrite_sstables_keyspace_compaction_task(cql, this_dc, rest_api):
-    task_tree_depth = 3
+    task_tree_depth = 2
     # upgrade sstables compaction
     check_compaction_task(cql, this_dc, rest_api, lambda keyspace, _: rest_api.send("GET", f"storage_service/keyspace_upgrade_sstables/{keyspace}"), "upgrade sstables compaction", task_tree_depth)
     # scrub sstables compaction