Raft: Fix coprocessor ignore new added RegionReadStatus thus cause inconsistent result (#10543)

close #10510, ref #10540

* Handling new added RegionReadStatus enum values for error response of coprocessor
  * Do not set the `default` branch for switch (e.status) so that the compiler will throw an error if there is not covered new enum values

Signed-off-by: JaySon-Huang <tshent@qq.com>

Author: JaySon-Huang

dbms/src/Flash/CoprocessorHandler.cpp

@@ -249,31 +249,8 @@ grpc::Status CoprocessorHandler<is_stream>::execute()
             response = cop_response;
         }
 
-        errorpb::Error * region_err;
-        switch (e.status)
-        {
-        case RegionException::RegionReadStatus::OTHER:
-        case RegionException::RegionReadStatus::BUCKET_EPOCH_NOT_MATCH:
-        case RegionException::RegionReadStatus::FLASHBACK:
-        case RegionException::RegionReadStatus::KEY_NOT_IN_REGION:
-        case RegionException::RegionReadStatus::TIKV_SERVER_ISSUE:
-        case RegionException::RegionReadStatus::READ_INDEX_TIMEOUT:
-        case RegionException::RegionReadStatus::NOT_LEADER:
-        case RegionException::RegionReadStatus::NOT_FOUND_TIKV:
-        case RegionException::RegionReadStatus::NOT_FOUND:
-            GET_METRIC(tiflash_coprocessor_request_error, reason_region_not_found).Increment();
-            region_err = response->mutable_region_error();
-            region_err->mutable_region_not_found()->set_region_id(cop_request->context().region_id());
-            break;
-        case RegionException::RegionReadStatus::EPOCH_NOT_MATCH:
-            GET_METRIC(tiflash_coprocessor_request_error, reason_epoch_not_match).Increment();
-            region_err = response->mutable_region_error();
-            region_err->mutable_epoch_not_match();
-            break;
-        default:
-            // should not happen
-            break;
-        }
+        setResponseByRegionException(response, e, cop_request->context().region_id());
+
         if constexpr (is_stream)
         {
             cop_writer->Write(stream_response);

dbms/src/Storages/KVStore/MultiRaft/RegionExecutionResult.h

@@ -14,7 +14,6 @@
 
 #pragma once
 
-#include <Storages/KVStore/Read/RegionException.h>
 #include <Storages/KVStore/Region_fwd.h>
 #include <Storages/KVStore/Types.h>
 

dbms/src/Storages/KVStore/Read/LearnerReadWorker.cpp

@@ -263,6 +263,10 @@ void LearnerReadWorker::recordReadIndexError(
     const LearnerReadSnapshot & regions_snapshot,
     RegionsReadIndexResult & read_index_result)
 {
+    // Now TiFlash set `unavailable_regions` by `read_index_result`, then `unavailable_regions` will check
+    // and throw `LockException` or `RegionException` in `tryThrowRegionException`. Then `CoprocessorHandler`
+    // or other similar logic handle the exceptions and retry the read request.
+
     // if size of batch_read_index_result is not equal with batch_read_index_req, there must be region_error/lock, find and return directly.
     for (auto & [region_id, resp] : read_index_result)
     {

dbms/src/Storages/KVStore/Read/RegionException.cpp

@@ -0,0 +1,65 @@
+// Copyright 2025 PingCAP, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <Common/TiFlashMetrics.h>
+#include <Storages/KVStore/Read/RegionException.h>
+#include <kvproto/coprocessor.pb.h>
+#include <kvproto/errorpb.pb.h>
+
+namespace DB
+{
+void setResponseByRegionException(coprocessor::Response * response, const RegionException & e, RegionID region_id)
+{
+    assert(response != nullptr);
+
+    errorpb::Error * region_err;
+    switch (e.status)
+    {
+    case RegionException::RegionReadStatus::OK:
+    case RegionException::RegionReadStatus::MEET_LOCK:
+        // Should not happen RegionException with RegionReadStatus::OK status
+        // And RegionReadStatus::MEET_LOCK should be handled in LockException
+        region_err = response->mutable_region_error();
+        region_err->set_message(
+            fmt::format("Unexpected RegionException with status {}", magic_enum::enum_name(e.status)));
+        region_err->mutable_region_not_found()->set_region_id(region_id);
+        break;
+    case RegionException::RegionReadStatus::OTHER:
+    case RegionException::RegionReadStatus::BUCKET_EPOCH_NOT_MATCH:
+    case RegionException::RegionReadStatus::FLASHBACK:
+    case RegionException::RegionReadStatus::KEY_NOT_IN_REGION:
+    case RegionException::RegionReadStatus::TIKV_SERVER_ISSUE:
+    case RegionException::RegionReadStatus::READ_INDEX_TIMEOUT:
+    case RegionException::RegionReadStatus::NOT_LEADER:
+    case RegionException::RegionReadStatus::NOT_FOUND_TIKV:
+    case RegionException::RegionReadStatus::NOT_FOUND:
+    case RegionException::RegionReadStatus::STALE_COMMAND:
+    case RegionException::RegionReadStatus::STORE_NOT_MATCH:
+        GET_METRIC(tiflash_coprocessor_request_error, reason_region_not_found).Increment();
+        region_err = response->mutable_region_error();
+        region_err->mutable_region_not_found()->set_region_id(region_id);
+        break;
+    case RegionException::RegionReadStatus::EPOCH_NOT_MATCH:
+        GET_METRIC(tiflash_coprocessor_request_error, reason_epoch_not_match).Increment();
+        region_err = response->mutable_region_error();
+        region_err->mutable_epoch_not_match();
+        // TODO: set current_regions when mutable epoch not match is hit
+        break;
+        // Notice: We need to handle all enum cases here to ensure correctly retrying.
+        // Do NOT add a default case here.
+        // default:
+        //     break;
+    }
+}
+} // namespace DB

dbms/src/Storages/KVStore/Read/RegionException.h

@@ -19,6 +19,11 @@
 
 #include <magic_enum.hpp>
 
+namespace coprocessor
+{
+class Response;
+}
+
 namespace DB
 {
 
@@ -59,4 +64,6 @@ class RegionException : public Exception
     RegionReadStatus status;
 };
 
+void setResponseByRegionException(coprocessor::Response * response, const RegionException & e, RegionID region_id);
+
 } // namespace DB

dbms/src/Storages/KVStore/tests/gtest_learner_read.cpp

@@ -17,13 +17,16 @@
 #include <Interpreters/Context.h>
 #include <Storages/KVStore/KVStore.h>
 #include <Storages/KVStore/Read/LearnerReadWorker.h>
+#include <Storages/KVStore/Read/RegionException.h>
 #include <Storages/KVStore/Region.h>
 #include <Storages/KVStore/Types.h>
 #include <Storages/PathPool.h>
 #include <Storages/RegionQueryInfo.h>
 #include <TestUtils/TiFlashTestBasic.h>
 #include <kvproto/kvrpcpb.pb.h>
 
+#include <magic_enum.hpp>
+
 namespace DB::tests
 {
 class LearnerReadTest : public ::testing::Test
@@ -261,4 +264,23 @@ try
 }
 CATCH
 
+TEST_F(LearnerReadTest, SetRespByRegionException)
+{
+    RegionID reg_id = 12345;
+    RegionException::UnavailableRegions regs{reg_id};
+
+    size_t num_enum_values = 0;
+    for (auto s : magic_enum::enum_values<RegionException::RegionReadStatus>())
+    {
+        auto tmp_regs = regs;
+        RegionException e(std::move(tmp_regs), s, "test");
+        coprocessor::Response response;
+        setResponseByRegionException(&response, e, reg_id);
+        // response must have region_error set
+        ASSERT_TRUE(response.has_region_error());
+        num_enum_values++;
+    }
+    LOG_INFO(Logger::get(), "SetRespByRegionException test passed with {} enum values.", num_enum_values);
+}
+
 } // namespace DB::tests