Avoid starving global merges with default space bucket deletions

Introduces a new top-most internal maintenance priority level and
changes bucket activation to use this level (still considered the
most important maintenance operation). Have global merge operations
use the old `VERY_HIGH` level. No other operations share this level.

5 files changed, 15 insertions, 10 deletions

diff --git a/storage/src/tests/distributor/maintenanceschedulertest.cpp b/storage/src/tests/distributor/maintenanceschedulertest.cpp
index f4b1b687158..bada963496d 100644
--- a/storage/src/tests/distributor/maintenanceschedulertest.cpp
+++ b/storage/src/tests/distributor/maintenanceschedulertest.cpp
@@ -36,7 +36,7 @@ MaintenanceSchedulerTest::SetUp()
 }
 
 TEST_F(MaintenanceSchedulerTest, priority_cleared_after_scheduled) {
-    _priorityDb->setPriority(PrioritizedBucket(makeDocumentBucket(BucketId(16, 1)), Priority::VERY_HIGH));
+    _priorityDb->setPriority(PrioritizedBucket(makeDocumentBucket(BucketId(16, 1)), Priority::HIGHEST));
     _scheduler->tick(MaintenanceScheduler::NORMAL_SCHEDULING_MODE);
     EXPECT_EQ("", _priorityDb->toString());
 }
@@ -48,20 +48,20 @@ TEST_F(MaintenanceSchedulerTest, operation_is_scheduled) {
               _operationStarter->toString());
 }
 
-TEST_F(MaintenanceSchedulerTest, no_operations_toschedule) {
+TEST_F(MaintenanceSchedulerTest, no_operations_to_schedule) {
     WaitTimeMs waitMs(_scheduler->tick(MaintenanceScheduler::NORMAL_SCHEDULING_MODE));
     EXPECT_EQ(WaitTimeMs(1), waitMs);
     EXPECT_EQ("", _operationStarter->toString());
 }
 
 TEST_F(MaintenanceSchedulerTest, suppress_low_priorities_in_emergency_mode) {
-    _priorityDb->setPriority(PrioritizedBucket(makeDocumentBucket(BucketId(16, 1)), Priority::HIGH));
-    _priorityDb->setPriority(PrioritizedBucket(makeDocumentBucket(BucketId(16, 2)), Priority::VERY_HIGH));
+    _priorityDb->setPriority(PrioritizedBucket(makeDocumentBucket(BucketId(16, 1)), Priority::VERY_HIGH));
+    _priorityDb->setPriority(PrioritizedBucket(makeDocumentBucket(BucketId(16, 2)), Priority::HIGHEST));
     EXPECT_EQ(WaitTimeMs(0), _scheduler->tick(MaintenanceScheduler::RECOVERY_SCHEDULING_MODE));
     EXPECT_EQ(WaitTimeMs(1), _scheduler->tick(MaintenanceScheduler::RECOVERY_SCHEDULING_MODE));
     EXPECT_EQ("Bucket(BucketSpace(0x0000000000000001), BucketId(0x4000000000000002)), pri 0\n",
               _operationStarter->toString());
-    EXPECT_EQ("PrioritizedBucket(Bucket(BucketSpace(0x0000000000000001), BucketId(0x4000000000000001)), pri HIGH)\n",
+    EXPECT_EQ("PrioritizedBucket(Bucket(BucketSpace(0x0000000000000001), BucketId(0x4000000000000001)), pri VERY_HIGH)\n",
               _priorityDb->toString());
 }
 
diff --git a/storage/src/tests/distributor/statecheckerstest.cpp b/storage/src/tests/distributor/statecheckerstest.cpp
index 00c1c7bb403..8f2a77572a4 100644
--- a/storage/src/tests/distributor/statecheckerstest.cpp
+++ b/storage/src/tests/distributor/statecheckerstest.cpp
@@ -698,14 +698,14 @@ TEST_F(StateCheckersTest, synchronize_and_move) {
             .clusterState("distributor:1 storage:4"));
 }
 
-TEST_F(StateCheckersTest, global_bucket_merges_have_high_priority_if_prioritization_enabled) {
+TEST_F(StateCheckersTest, global_bucket_merges_have_very_high_priority_if_prioritization_enabled) {
     runAndVerify<SynchronizeAndMoveStateChecker>(
             CheckerParams().expect(
                             "[Synchronizing buckets with different checksums "
                             "node(idx=0,crc=0x1,docs=1/1,bytes=1/1,trusted=false,active=false,ready=false), "
                             "node(idx=1,crc=0x2,docs=2/2,bytes=2/2,trusted=false,active=false,ready=false)] "
                             "(pri 115) "
-                            "(scheduling pri HIGH)")
+                            "(scheduling pri VERY_HIGH)")
                     .bucketInfo("0=1,1=2")
                     .bucket_space(document::FixedBucketSpaces::global_space())
                     .includeSchedulingPriority(true)
diff --git a/storage/src/vespa/storage/distributor/maintenance/maintenancepriority.h b/storage/src/vespa/storage/distributor/maintenance/maintenancepriority.h
index a2fea0e566d..11bde9b1986 100644
--- a/storage/src/vespa/storage/distributor/maintenance/maintenancepriority.h
+++ b/storage/src/vespa/storage/distributor/maintenance/maintenancepriority.h
@@ -14,6 +14,7 @@ public:
         MEDIUM,
         HIGH,
         VERY_HIGH,
+        HIGHEST,
         PRIORITY_LIMIT
     };
 
@@ -25,6 +26,7 @@ public:
         case MEDIUM: return "MEDIUM";
         case HIGH: return "HIGH";
         case VERY_HIGH: return "VERY_HIGH";
+        case HIGHEST: return "HIGHEST";
         default: return "INVALID";
         }
     }
diff --git a/storage/src/vespa/storage/distributor/maintenance/maintenancescheduler.cpp b/storage/src/vespa/storage/distributor/maintenance/maintenancescheduler.cpp
index 5481559cf5a..6e502a0ef39 100644
--- a/storage/src/vespa/storage/distributor/maintenance/maintenancescheduler.cpp
+++ b/storage/src/vespa/storage/distributor/maintenance/maintenancescheduler.cpp
@@ -61,7 +61,7 @@ bool
 MaintenanceScheduler::possibleToScheduleInEmergency(
         const PrioritizedBucket& bucket) const
 {
-    return bucket.moreImportantThan(MaintenancePriority::HIGH);
+    return bucket.moreImportantThan(MaintenancePriority::VERY_HIGH);
 }
 
 void
@@ -84,6 +84,8 @@ MaintenanceScheduler::convertToOperationPriority(MaintenancePriority::Priority p
     case MaintenancePriority::HIGH:
         return OperationStarter::Priority(50);
     case MaintenancePriority::VERY_HIGH:
+        return OperationStarter::Priority(30);
+    case MaintenancePriority::HIGHEST:
         return OperationStarter::Priority(0);
     default:
         LOG_ABORT("should not be reached");
diff --git a/storage/src/vespa/storage/distributor/statecheckers.cpp b/storage/src/vespa/storage/distributor/statecheckers.cpp
index a2342545ae1..796a0434ed4 100644
--- a/storage/src/vespa/storage/distributor/statecheckers.cpp
+++ b/storage/src/vespa/storage/distributor/statecheckers.cpp
@@ -868,7 +868,8 @@ SynchronizeAndMoveStateChecker::check(StateChecker::Context& c)
         } else {
             // Since the default bucket space has a dependency on the global bucket space,
             // we prioritize scheduling of merges to global buckets over those for default buckets.
-            schedPri = MaintenancePriority::HIGH;
+            // We also prioritize these above bucket deletions for the default space to avoid starvation.
+            schedPri = MaintenancePriority::VERY_HIGH;
             op->setPriority(c.distributorConfig.getMaintenancePriorities().mergeGlobalBuckets);
         }
 
@@ -1119,7 +1120,7 @@ BucketStateStateChecker::check(StateChecker::Context& c)
         op->setPriority(c.distributorConfig.getMaintenancePriorities().activateWithExistingActive);
     }
     op->setDetailedReason(reason.str());
-    return Result::createStoredResult(std::move(op), MaintenancePriority::VERY_HIGH);
+    return Result::createStoredResult(std::move(op), MaintenancePriority::HIGHEST);
 }
 
 bool