[BACKPORT 2.21.0][#22057] docdb: converted ysql_skip_row_lock_for_upd…

…ate to auto-flag

Summary:
Combination of two commits: decb104111 + 6de97906 introduced a compatibility issue between pre-decb104111 and post-6de97906 tserver versions if they are running at the same time during upgrade.

With the post-6de97906 logic `WriteQuery::DoCompleteExecute` when isolation level is not `SERIALIZABLE_ISOLATION` it can add WRITE_OP that contains both `write_pairs` and `read_pairs` to the Raft log. Corresponding `read_pair` will have key set to encoded row key and value set to `KeyEntryTypeAsChar::kNullLow`.

This WRITE_OP is then processed by `TransactionalWriter::Apply` and corresponding intents are generated and written to the intents DB.

In post-6de97906 version `TransactionalWriter::Apply` processes such `read_pair` and as a result generates intent `<row> -> kNullLow` of type `[kStrongRead]`.
But in pre-decb104111 version `TransactionalWriter::Apply` processes such `read_pair` in a different way and generates intent `<row> -> kNullLow` of type `[kStrongRead, kStrongWrite]`.

Then `ApplyIntentsContext::Entry` processes intents and in pre-decb104111 version due to presence of `kStrongWrite` type this intent gets written into regular DB and results in the following record:
`<row> -> kNullLow`, which is incorrect regular DB record. Effect of handing this record by DocDB is different depending on WHERE filter and presence of aggregation, it may result in either affected rows be not visible by the user statement or visible but with non-PK columns set to null.

Given that `TransactionalWriter::Apply` only writes `read_pairs` for non-SERIALIZABLE_ISOLATION when new logic is enabled by `ysql_skip_row_lock_for_update`, the solution is to convert it to an auto-flag, so new logic is only enabled after all nodes are on the new version.

Also added `PgsqlWriteOperation::use_row_lock_for_update_` which is initialized in constructor to avoid changing behaviour in context of the same `PgsqlWriteOperation` instance (since flag is now runtime and the auto-flag will change at runtime from false to true).
Jira: DB-10979

Original commit: 987163c / D34733

Test Plan: Run TPCC workload in parallel with upgrade from 2.18.7.0-b38 to build with this fix incorporated.

Reviewers: smishra, hsunder, dmitry, timur, rthallam

Reviewed By: rthallam

Subscribers: aaruj, ybase, yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D34916

src/yb/docdb/pgsql_operation.cc

@@ -118,8 +118,7 @@ DEFINE_RUNTIME_bool(ysql_enable_pack_full_row_update, false,
 DEFINE_RUNTIME_PREVIEW_bool(ysql_use_packed_row_v2, false,
                             "Whether to use packed row V2 when row packing is enabled.");
 
-DEFINE_NON_RUNTIME_bool(
-    ysql_skip_row_lock_for_update, false,
+DEFINE_RUNTIME_AUTO_bool(ysql_skip_row_lock_for_update, kExternal, true, false,
     "By default DocDB operations for YSQL take row-level locks. If set to true, DocDB will instead "
     "take finer column-level locks instead of locking the whole row. This may cause issues with "
     "data integrity for operations with implicit dependencies between columns.");
@@ -798,6 +797,15 @@ class PgsqlWriteOperation::RowPackContext {
 
 //--------------------------------------------------------------------------------------------------
 
+PgsqlWriteOperation::PgsqlWriteOperation(
+    std::reference_wrapper<const PgsqlWriteRequestPB> request, DocReadContextPtr doc_read_context,
+    const TransactionOperationContext& txn_op_context, rpc::Sidecars* sidecars)
+    : DocOperationBase(request),
+      doc_read_context_(std::move(doc_read_context)),
+      txn_op_context_(txn_op_context),
+      sidecars_(sidecars),
+      ysql_skip_row_lock_for_update_(FLAGS_ysql_skip_row_lock_for_update) {}
+
 Status PgsqlWriteOperation::Init(PgsqlResponsePB* response) {
   // Initialize operation inputs.
   response_ = response;
@@ -1506,7 +1514,7 @@ Status PgsqlWriteOperation::GetDocPaths(GetDocPathsMode mode,
   const auto is_update = request_.stmt_type() == PgsqlWriteRequestPB::PGSQL_UPDATE;
   switch (mode) {
     case GetDocPathsMode::kLock: {
-      if (PREDICT_FALSE(FLAGS_ysql_skip_row_lock_for_update) && is_update &&
+      if (PREDICT_FALSE(ysql_skip_row_lock_for_update_) && is_update &&
           !NewValuesHaveExpression(request_)) {
         DocKeyColumnPathBuilderHolder holder(encoded_doc_key_.as_slice());
         paths->emplace_back(holder.builder().Build(dockv::SystemColumnIds::kLivenessColumn));
@@ -1532,7 +1540,7 @@ Status PgsqlWriteOperation::GetDocPaths(GetDocPathsMode mode,
       return Status::OK();
     }
     case GetDocPathsMode::kStrongReadIntents: {
-      if (!is_update || PREDICT_FALSE(FLAGS_ysql_skip_row_lock_for_update)) {
+      if (!is_update || PREDICT_FALSE(ysql_skip_row_lock_for_update_)) {
         return Status::OK();
       }
       break;

src/yb/docdb/pgsql_operation.h

@@ -47,12 +47,7 @@ class PgsqlWriteOperation :
   PgsqlWriteOperation(std::reference_wrapper<const PgsqlWriteRequestPB> request,
                       DocReadContextPtr doc_read_context,
                       const TransactionOperationContext& txn_op_context,
-                      rpc::Sidecars* sidecars)
-      : DocOperationBase(request),
-        doc_read_context_(std::move(doc_read_context)),
-        txn_op_context_(txn_op_context),
-        sidecars_(sidecars) {
-  }
+                      rpc::Sidecars* sidecars);
 
   // Initialize PgsqlWriteOperation. Content of request will be swapped out by the constructor.
   Status Init(PgsqlResponsePB* response);
@@ -142,6 +137,7 @@ class PgsqlWriteOperation :
   int64_t result_rows_ = 0;
   WriteBufferPos row_num_pos_;
   WriteBuffer* write_buffer_ = nullptr;
+  const bool ysql_skip_row_lock_for_update_;
 };
 
 class PgsqlReadOperation : public DocExprExecutor {