Use priority class LOW for generated GC operations

Avoids starvation of merge (and split) operations when GC ops are
preempted on heavily loaded content nodes.

This fixes #3249

2 files changed, 14 insertions, 4 deletions

diff --git a/storage/src/tests/distributor/statecheckerstest.cpp b/storage/src/tests/distributor/statecheckerstest.cpp
index fbaa8a7662d..29c922248e7 100644
--- a/storage/src/tests/distributor/statecheckerstest.cpp
+++ b/storage/src/tests/distributor/statecheckerstest.cpp
@@ -83,6 +83,7 @@ struct StateCheckersTest : public CppUnit::TestFixture,
     void inhibitBucketDeactivationIfDisabledInConfig();
     void retiredNodesOutOfSyncAreMerged();
     void testGarbageCollection();
+    void gc_ops_are_prioritized_with_low_priority_category();
     void gcInhibitedWhenIdealNodeInMaintenance();
     void testNoRemoveWhenIdealNodeInMaintenance();
     void testStepwiseJoinForSmallBucketsWithoutSiblings();
@@ -186,7 +187,8 @@ struct StateCheckersTest : public CppUnit::TestFixture,
                                       uint32_t nowTimestamp,
                                       uint32_t checkInterval,
                                       uint32_t lastChangeTime = 0,
-                                      bool includePriority = false);
+                                      bool includePriority = false,
+                                      bool includeSchedulingPri = false);
 
     std::string testSplit(uint32_t splitCount,
                           uint32_t splitSize,
@@ -321,6 +323,7 @@ struct StateCheckersTest : public CppUnit::TestFixture,
     CPPUNIT_TEST(inhibitBucketActivationIfDisabledInConfig);
     CPPUNIT_TEST(inhibitBucketDeactivationIfDisabledInConfig);
     CPPUNIT_TEST(testGarbageCollection);
+    CPPUNIT_TEST(gc_ops_are_prioritized_with_low_priority_category);
     CPPUNIT_TEST(gcInhibitedWhenIdealNodeInMaintenance);
     CPPUNIT_TEST(testNoRemoveWhenIdealNodeInMaintenance);
     CPPUNIT_TEST(testStepwiseJoinForSmallBucketsWithoutSiblings);
@@ -1458,7 +1461,7 @@ StateCheckersTest::inhibitBucketDeactivationIfDisabledInConfig()
 std::string StateCheckersTest::testGarbageCollection(
         uint32_t prevTimestamp, uint32_t nowTimestamp,
         uint32_t checkInterval, uint32_t lastChangeTime,
-        bool includePriority)
+        bool includePriority, bool includeSchedulingPri)
 {
     BucketDatabase::Entry e(document::BucketId(17, 0));
     e.getBucketInfo().addNode(BucketCopy(prevTimestamp, 0,
@@ -1475,7 +1478,7 @@ std::string StateCheckersTest::testGarbageCollection(
             e.getBucketId());
     getClock().setAbsoluteTimeInSeconds(nowTimestamp);
     return testStateChecker(checker, c, false, PendingMessage(),
-                            includePriority);
+                            includePriority, includeSchedulingPri);
 }
 
 void
@@ -1527,6 +1530,13 @@ StateCheckersTest::testGarbageCollection()
             testGarbageCollection(3850, 4000, 300, 1));
 }
 
+void StateCheckersTest::gc_ops_are_prioritized_with_low_priority_category() {
+    CPPUNIT_ASSERT_EQUAL(
+            std::string("[Needs garbage collection: Last check at 3, current time 4000, "
+                        "configured interval 300] (scheduling pri LOW)"),
+            testGarbageCollection(3, 4000, 300, 1, false, true));
+}
+
 /**
  * When a node is in maintenance, we want to do our best to avoid any unneeded
  * changes to the bucket replicas' states, as this will require re-syncing of
diff --git a/storage/src/vespa/storage/distributor/statecheckers.cpp b/storage/src/vespa/storage/distributor/statecheckers.cpp
index f81b6d2bcb6..701a147a56f 100644
--- a/storage/src/vespa/storage/distributor/statecheckers.cpp
+++ b/storage/src/vespa/storage/distributor/statecheckers.cpp
@@ -1140,7 +1140,7 @@ GarbageCollectionStateChecker::check(Context& c)
         op->setPriority(c.distributorConfig.getMaintenancePriorities()
                         .garbageCollection);
         op->setDetailedReason(reason.c_str());
-        return Result::createStoredResult(std::move(op), MaintenancePriority::MEDIUM);
+        return Result::createStoredResult(std::move(op), MaintenancePriority::LOW);
     } else {
         return Result::noMaintenanceNeeded();
     }