Reply with retriable errors on stale snapshot

5 files changed, 23 insertions, 7 deletions

diff --git a/ydb/core/kqp/executer_actor/kqp_data_executer.cpp b/ydb/core/kqp/executer_actor/kqp_data_executer.cpp
index 27220f1d36a..c85dc3882cf 100644
--- a/ydb/core/kqp/executer_actor/kqp_data_executer.cpp
+++ b/ydb/core/kqp/executer_actor/kqp_data_executer.cpp
@@ -593,6 +593,11 @@ private:
             }
             case NKikimrTxDataShard::TEvProposeTransactionResult::BAD_REQUEST: {
                 Counters->TxProxyMon->TxResultCancelled->Inc();
+                if (HasMissingSnapshotError(result)) {
+                    auto issue = YqlIssue({}, TIssuesIds::KIKIMR_PRECONDITION_FAILED);
+                    AddDataShardErrors(result, issue);
+                    return ReplyErrorAndDie(Ydb::StatusIds::PRECONDITION_FAILED, issue);
+                }
                 auto issue = YqlIssue({}, TIssuesIds::KIKIMR_BAD_REQUEST);
                 AddDataShardErrors(result, issue);
                 return ReplyErrorAndDie(Ydb::StatusIds::BAD_REQUEST, issue);
@@ -2284,6 +2289,15 @@ private:
         }
     }
 
+    static bool HasMissingSnapshotError(const NKikimrTxDataShard::TEvProposeTransactionResult& result) {
+        for (const auto& err : result.GetError()) {
+            if (err.GetKind() == NKikimrTxDataShard::TError::SNAPSHOT_NOT_EXIST) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     static void AddDataShardErrors(const NKikimrTxDataShard::TEvProposeTransactionResult& result, TIssue& issue) {
         for (const auto &err : result.GetError()) {
             issue.AddSubIssue(new TIssue(TStringBuilder()
diff --git a/ydb/core/kqp/ut/tx/kqp_mvcc_ut.cpp b/ydb/core/kqp/ut/tx/kqp_mvcc_ut.cpp
index d1559a7a353..f2ed5f7bdc7 100644
--- a/ydb/core/kqp/ut/tx/kqp_mvcc_ut.cpp
+++ b/ydb/core/kqp/ut/tx/kqp_mvcc_ut.cpp
@@ -55,6 +55,8 @@ Y_UNIT_TEST_SUITE(KqpSnapshotRead) {
                     return issue.GetMessage().Contains("stale snapshot");
                 }), result.GetIssues().ToString());
 
+            UNIT_ASSERT_VALUES_EQUAL(result.GetStatus(), EStatus::PRECONDITION_FAILED);
+
             caught = true;
             break;
         } while (TInstant::Now() < deadline);
diff --git a/ydb/core/tx/datashard/check_data_tx_unit.cpp b/ydb/core/tx/datashard/check_data_tx_unit.cpp
index deb9f1f903a..53172927bc5 100644
--- a/ydb/core/tx/datashard/check_data_tx_unit.cpp
+++ b/ydb/core/tx/datashard/check_data_tx_unit.cpp
@@ -104,7 +104,7 @@ EExecutionStatus TCheckDataTxUnit::Execute(TOperation::TPtr op,
                 << " at " << DataShard.TabletID();
 
             BuildResult(op, NKikimrTxDataShard::TEvProposeTransactionResult::BAD_REQUEST)
-                ->AddError(NKikimrTxDataShard::TError::BAD_ARGUMENT, err);
+                ->AddError(NKikimrTxDataShard::TError::SNAPSHOT_NOT_EXIST, err);
             op->Abort(EExecutionUnitKind::FinishPropose);
 
             LOG_ERROR_S(ctx, NKikimrServices::TX_DATASHARD, err);
diff --git a/ydb/core/tx/datashard/datashard__read_iterator.cpp b/ydb/core/tx/datashard/datashard__read_iterator.cpp
index 514b5371cbe..e2fecfff623 100644
--- a/ydb/core/tx/datashard/datashard__read_iterator.cpp
+++ b/ydb/core/tx/datashard/datashard__read_iterator.cpp
@@ -904,7 +904,7 @@ public:
             if (!snapshotFound) {
                 SetStatusError(
                     Result->Record,
-                    Ydb::StatusIds::NOT_FOUND,
+                    Ydb::StatusIds::PRECONDITION_FAILED,
                     TStringBuilder() << "Table id " << tableId << " lost snapshot at "
                          << state.ReadVersion << " shard " << Self->TabletID()
                          << " with lowWatermark " << Self->GetSnapshotManager().GetLowWatermark()
@@ -1116,7 +1116,7 @@ public:
             if (!snapshotFound) {
                 SetStatusError(
                     Result->Record,
-                    Ydb::StatusIds::NOT_FOUND,
+                    Ydb::StatusIds::PRECONDITION_FAILED,
                     TStringBuilder() << "Table id " << tableId << " has no snapshot at "
                          << state.ReadVersion << " shard " << Self->TabletID()
                          << " with lowWatermark " << Self->GetSnapshotManager().GetLowWatermark()
@@ -1354,7 +1354,7 @@ private:
         if (!Self->GetSnapshotManager().FindAvailable(snapshotKey) && !allowMvcc) {
             SetStatusError(
                 Result->Record,
-                Ydb::StatusIds::ABORTED,
+                Ydb::StatusIds::PRECONDITION_FAILED,
                 TStringBuilder() << "Table id " << tableId << " lost snapshot at "
                      << state.ReadVersion << " shard " << Self->TabletID()
                      << " with lowWatermark " << Self->GetSnapshotManager().GetLowWatermark()
@@ -1746,7 +1746,7 @@ public:
         if (!Self->GetSnapshotManager().FindAvailable(snapshotKey) && !allowMvcc) {
             SetStatusError(
                 Result->Record,
-                Ydb::StatusIds::ABORTED,
+                Ydb::StatusIds::PRECONDITION_FAILED,
                 TStringBuilder() << "Table id " << tableId << " lost snapshot at "
                      << state.ReadVersion << " shard " << Self->TabletID()
                      << " with lowWatermark " << Self->GetSnapshotManager().GetLowWatermark()
@@ -2007,7 +2007,7 @@ void TDataShard::Handle(TEvDataShard::TEvRead::TPtr& ev, const TActorContext& ct
                 // check if there is MVCC version and maybe wait
                 if (readVersion < GetSnapshotManager().GetLowWatermark()) {
                     replyWithError(
-                        Ydb::StatusIds::NOT_FOUND,
+                        Ydb::StatusIds::PRECONDITION_FAILED,
                         TStringBuilder() << "MVCC read " << readVersion
                             << " bellow low watermark " << GetSnapshotManager().GetLowWatermark());
                     return;
diff --git a/ydb/core/tx/datashard/datashard_ut_read_iterator.cpp b/ydb/core/tx/datashard/datashard_ut_read_iterator.cpp
index 11db2999f48..244a31ccda3 100644
--- a/ydb/core/tx/datashard/datashard_ut_read_iterator.cpp
+++ b/ydb/core/tx/datashard/datashard_ut_read_iterator.cpp
@@ -1848,7 +1848,7 @@ Y_UNIT_TEST_SUITE(DataShardReadIterator) {
         AddKeyQuery(*request, {3, 3, 3});
         auto readResult = helper.SendRead("table-1", request.release());
         const auto& record = readResult->Record;
-        UNIT_ASSERT_VALUES_EQUAL(record.GetStatus().GetCode(), Ydb::StatusIds::NOT_FOUND);
+        UNIT_ASSERT_VALUES_EQUAL(record.GetStatus().GetCode(), Ydb::StatusIds::PRECONDITION_FAILED);
     }
 
     Y_UNIT_TEST(ShouldNotReadHeadFromFollower) {