These commits are when the Protocol Buffers files have changed: (only the last 100 relevant commits are shown)
| Commit: | 4d1d27c | |
|---|---|---|
| Author: | Timur Yusupov | |
| Committer: | Timur Yusupov | |
[BACKPORT 2025.2.4][#31930,#31241] docdb: Fix RBS vs ConfigChange races Summary: There is a possibility of RBS vs ConfigChange races, for example: 1. Raft config for the tablet has nodes A (leader), B, C. 2. D is added to Raft config, CHANGE_CONFIG operation is committed and applied on A, B, C 3. RBS A -> D started 4. D is removed from Raft config, CHANGE_CONFIG operation is committed and applied on leader 5. RBS A -> D downloads WAL and completed, D has the latest committed Raft config **Expected result:** orphaned tablet replica on D should be deleted. **Actual result:** we have an orphaned tablet replica (lagging follower) on D that is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader. In order to fix that, behaviour inside `MasterHeartbeatServiceImpl::ProcessTabletReport` is changed to delete a tablet replica which is no longer part of the committed Raft config and Raft config that added that replica is no longer pending (either committed or aborted). The latter condition avoids deleting the newly RBSed tablet replica that is being added to the tablet Raft group. The following logic is implemented to support that: 1. Once tablet leader decides to start RBS replica on another tserver, it will include the current pending Raft config op id (both term and index) into `StartRemoteBootstrapRequestPB` or empty op id when no config change is pending. 2. The bootstrapping replica persists this op id in its consensus metadata as `pending_config_op_id_from_rbs`. It is cleared once the replica's last committed op id either advances to a higher term, or its index reaches/passes the stored pending op id's index - i.e. once the original `CHANGE_CONFIG` operation can no longer be pending (it has either committed or been aborted). 3. `ReportedTabletPB::pending_config_op_id` is added to tserver->master heartbeats. Its value is whichever is set: the replica's currently active pending config op id (a config change in progress on the replica itself) or the pending_config_op_id_from_rbs from (2). 4. When master leader receives a tablet report from a replica that is *not* in the committed Raft config last known by the master, the master triggers `DeleteTabletRequestPB` to that replica if both: a. The reported committed Raft config op id index is <= the committed Raft config op id index last known by master for this tablet. b. Either the report carries no pending op id (empty/missing), or the master can prove it is no longer pending: if the pending op id's term is strictly less than the `current_term` in the master's last-known committed consensus state for this tablet, that `CHANGE_CONFIG` must have already been aborted or committed, and condition (4.a) alone is sufficient. 5. TServer will include tablet in next heartbeat in case DeleteTablet failed due to stale cas_config_opid_index_less_or_equal (this scenario is covered by TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart). Also update master-side of CloneTablet operation to seed the target tablet's committed_consensus_state peers on the master from the source tablet's config so that when the cloned replicas heartbeat back, `ProcessTabletReportBatch` sees them as part of the expected Raft config and does not tombstone them. Added several unit-tests for other RBS vs ConfigChange race scenarios. They are also fixed by the implemented change. Also renamed `RaftConfigPB.opid_index` to `committed_op_index` in order to reflect the actual purpose of this field. **Upgrade/Rollback safety:** New logic at master-side is gated by a new `use_tablet_report_pending_config_op_id` auto flag. Until the whole cluster is fully upgraded, master won't rely on newly added `ReportedTabletPB::pending_config_op_id` field. Original commit: 54c3d4ca73c593b364abf3f9462e788d32a20f53 / D52759 Test Plan: Run the following tests for asan/tsan/debug/relelase for 20 iterations each: - TabletSplitITest.SplitWithParentTabletRbsFromFollower - TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRace - TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart - TabletReplacementITest.TombstoneEvictedReplicaAfterAbortedAddServer - covers https://github.com/yugabyte/yugabyte-db/issues/31241 - TabletReplacementITest.DontDeleteNewReplicaInPendingConfig - TabletReplacementITest.DontDeleteNewReplicaInPendingConfigAfterRbsFromFollowerRf5 Reviewers: zdrudi Reviewed By: zdrudi Subscribers: ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D55227
| Commit: | b3f5303 | |
|---|---|---|
| Author: | Timur Yusupov | |
| Committer: | Timur Yusupov | |
[BACKPORT 2026.1.0][#31930,#31241] docdb: Fix RBS vs ConfigChange races Summary: There is a possibility of RBS vs ConfigChange races, for example: 1. Raft config for the tablet has nodes A (leader), B, C. 2. D is added to Raft config, CHANGE_CONFIG operation is committed and applied on A, B, C 3. RBS A -> D started 4. D is removed from Raft config, CHANGE_CONFIG operation is committed and applied on leader 5. RBS A -> D downloads WAL and completed, D has the latest committed Raft config **Expected result:** orphaned tablet replica on D should be deleted. **Actual result:** we have an orphaned tablet replica (lagging follower) on D that is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader. In order to fix that, behaviour inside `MasterHeartbeatServiceImpl::ProcessTabletReport` is changed to delete a tablet replica which is no longer part of the committed Raft config and Raft config that added that replica is no longer pending (either committed or aborted). The latter condition avoids deleting the newly RBSed tablet replica that is being added to the tablet Raft group. The following logic is implemented to support that: 1. Once tablet leader decides to start RBS replica on another tserver, it will include the current pending Raft config op id (both term and index) into `StartRemoteBootstrapRequestPB` or empty op id when no config change is pending. 2. The bootstrapping replica persists this op id in its consensus metadata as `pending_config_op_id_from_rbs`. It is cleared once the replica's last committed op id either advances to a higher term, or its index reaches/passes the stored pending op id's index - i.e. once the original `CHANGE_CONFIG` operation can no longer be pending (it has either committed or been aborted). 3. `ReportedTabletPB::pending_config_op_id` is added to tserver->master heartbeats. Its value is whichever is set: the replica's currently active pending config op id (a config change in progress on the replica itself) or the pending_config_op_id_from_rbs from (2). 4. When master leader receives a tablet report from a replica that is *not* in the committed Raft config last known by the master, the master triggers `DeleteTabletRequestPB` to that replica if both: a. The reported committed Raft config op id index is <= the committed Raft config op id index last known by master for this tablet. b. Either the report carries no pending op id (empty/missing), or the master can prove it is no longer pending: if the pending op id's term is strictly less than the `current_term` in the master's last-known committed consensus state for this tablet, that `CHANGE_CONFIG` must have already been aborted or committed, and condition (4.a) alone is sufficient. 5. TServer will include tablet in next heartbeat in case DeleteTablet failed due to stale cas_config_opid_index_less_or_equal (this scenario is covered by TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart). Also update master-side of CloneTablet operation to seed the target tablet's committed_consensus_state peers on the master from the source tablet's config so that when the cloned replicas heartbeat back, `ProcessTabletReportBatch` sees them as part of the expected Raft config and does not tombstone them. Added several unit-tests for other RBS vs ConfigChange race scenarios. They are also fixed by the implemented change. Also renamed `RaftConfigPB.opid_index` to `committed_op_index` in order to reflect the actual purpose of this field. **Upgrade/Rollback safety:** New logic at master-side is gated by a new `use_tablet_report_pending_config_op_id` auto flag. Until the whole cluster is fully upgraded, master won't rely on newly added `ReportedTabletPB::pending_config_op_id` field. Original commit: 54c3d4ca73c593b364abf3f9462e788d32a20f53 / D52759 Test Plan: Run the following tests for asan/tsan/debug/relelase for 20 iterations each: - TabletSplitITest.SplitWithParentTabletRbsFromFollower - TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRace - TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart - TabletReplacementITest.TombstoneEvictedReplicaAfterAbortedAddServer - covers https://github.com/yugabyte/yugabyte-db/issues/31241 - TabletReplacementITest.DontDeleteNewReplicaInPendingConfig - TabletReplacementITest.DontDeleteNewReplicaInPendingConfigAfterRbsFromFollowerRf5 Reviewers: zdrudi Reviewed By: zdrudi Subscribers: ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D55224
| Commit: | cec2be3 | |
|---|---|---|
| Author: | Timur Yusupov | |
| Committer: | Timur Yusupov | |
[BACKPORT 2025.2.4][#27056] docdb: Fixed tablet split vs RBS from the follower race Summary: There is a possibility of tablet split vs RBS from follower race: 1. Parent tablet leader peers A-C accept a SPLIT_OP (op_id: 1.4). Leader (A) and 1st follower (B) apply SPLIT_OP, 2nd follower (C) doesn't apply it yet. 2. Parent tablet leader (node A) accepts CHANGE_CONFIG_OP (op_id: 1.5) to add a fourth peer (D) but doesn't apply it yet. 3. Parent tablet 2nd follower (C) still hasn't yet applied SPLIT_OP (1.4). 4. RBS for parent tablet peer (D) starts from the follower (C) and tablet metadata (tablet_data_state == TABLET_DATA_READY) is downloaded. 5. Parent tablet peers A-C completed applying the SPLIT_OP (1.4), child tablets have Raft config with 3 peers. 6. Parent tablet peers A-C apply CHANGE_CONFIG_OP (1.5) and now have committed Raft config with 4 peers. 7. Parent tablet peer D does local bootstrap and replays SPLIT_OP (1.4) as part of bootstrap. Due to tablet_data_state is TABLET_DATA_READY but not TABLET_DATA_SPLIT_COMPLETED replay does SPLIT_OP apply and creates child tablet peer. After that, 4th child tablet peer (D) is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader. This change fixes this race by rejecting RBS from the follower that is in progress of applying SPLIT_OP and RBS attempt will be retried later. Original commit: 78af3a208a6bfc005eedc0ed6e410d22f4d24758 / D48853 **Upgrade/Rollback safety:** New error code will be printed by old nodes as just number in case of RBS failure during upgrade but this is safe. Test Plan: TabletSplitITest.SplitWithParentTabletRbsFromFollower, TabletSplitITest.SplitWithParentTabletMove, RemoteBootstrapsFromNodeWithUncommittedSplitOp - 30 runs per each of asan/tsan/debug/release builds Jenkins: urgent Reviewers: arybochkin, #db-approvers Reviewed By: arybochkin, #db-approvers Subscribers: hbhanawat, svc_phabricator, ybase, zdrudi Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D55181
| Commit: | 533f6ee | |
|---|---|---|
| Author: | Timur Yusupov | |
| Committer: | Timur Yusupov | |
[BACKPORT 2026.1.0][#27056] docdb: Fixed tablet split vs RBS from the follower race Summary: There is a possibility of tablet split vs RBS from follower race: 1. Parent tablet leader peers A-C accept a SPLIT_OP (op_id: 1.4). Leader (A) and 1st follower (B) apply SPLIT_OP, 2nd follower (C) doesn't apply it yet. 2. Parent tablet leader (node A) accepts CHANGE_CONFIG_OP (op_id: 1.5) to add a fourth peer (D) but doesn't apply it yet. 3. Parent tablet 2nd follower (C) still hasn't yet applied SPLIT_OP (1.4). 4. RBS for parent tablet peer (D) starts from the follower (C) and tablet metadata (tablet_data_state == TABLET_DATA_READY) is downloaded. 5. Parent tablet peers A-C completed applying the SPLIT_OP (1.4), child tablets have Raft config with 3 peers. 6. Parent tablet peers A-C apply CHANGE_CONFIG_OP (1.5) and now have committed Raft config with 4 peers. 7. Parent tablet peer D does local bootstrap and replays SPLIT_OP (1.4) as part of bootstrap. Due to tablet_data_state is TABLET_DATA_READY but not TABLET_DATA_SPLIT_COMPLETED replay does SPLIT_OP apply and creates child tablet peer. After that, 4th child tablet peer (D) is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader. This change fixes this race by rejecting RBS from the follower that is in progress of applying SPLIT_OP and RBS attempt will be retried later. Original commit: 78af3a208a6bfc005eedc0ed6e410d22f4d24758 / D48853 **Upgrade/Rollback safety:** New error code will be printed by old nodes as just number in case of RBS failure during upgrade but this is safe. Test Plan: TabletSplitITest.SplitWithParentTabletRbsFromFollower, TabletSplitITest.SplitWithParentTabletMove, RemoteBootstrapsFromNodeWithUncommittedSplitOp - 30 runs per each of asan/tsan/debug/release builds Jenkins: urgent, all tests Reviewers: arybochkin, #db-approvers Reviewed By: arybochkin, #db-approvers Subscribers: hbhanawat, svc_phabricator, zdrudi, ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D55184
| Commit: | 5db3d9a | |
|---|---|---|
| Author: | Bvsk Patnaik | |
| Committer: | Bvsk Patnaik | |
[#32335] YSQL: Pass single read time option from pggate to pg client session Summary: #### Problem Prior to this revision, multiple read time options may be set on PerformOptionsPB. It is unclear which read time option should be preferred. Example: - Parallel query may set ENSURE_READ_TIME_IS_SET read_time_manipulation. - GUC yb_read_after_commit_visibility=relaxed sets clamp_uncertainty_window option. Both options are set on the proto and the clamp option should be preferred. This is not obvious. #### Approach 1. Decide the preference in pggate itself. 2. Send only a single read time option in the perform RPC. This simplifies the contract between pggate and pg client session. Additionally: 1. Collect read time options logic into PgTxnManager::SetReadTimeOptions 2. Add appropriate validation checks for incompatible read time options. 3. Document this in pggate/README in section named Read point selection section. 4. Rename catalog session to legacy catalog session since plain sessions are used for catalog reads with newer concurrent DDL feature. 5. On similar lines, rename kDDL sessions to kAutonomousDDL. #### Some notes: 1. Follower reads are only applicable to read only transactions. And serializable never occurs in read only transactions. Therefore, follower reads and serializable do not co-occur. 2. Previously, parallel scans overwrote RESTART flag to ENSURE_READ_TIME_IS_SET. Instead, RESTART should take precedence because RESTART picks a read time on pg client session as well. 3. Previously, clamp and deferred options were ignored if any of the writes within the txn are non transactional. Now, the perform RPC must have a non transactional write to ignore clamp and deferred options. #### Upgrade/Rollback safety Perform RPC is intra node and therefore the client and the server are co-versioned. Test Plan: Jenkins ./yb_build.sh release --cxx-test pg_read_time-test Reviewers: pjain, smishra, sanketh, bkolagani Reviewed By: pjain Subscribers: yql, ybase Differential Revision: https://phorge.dev.yugabyte.com/D54921
The documentation is generated from this commit.
| Commit: | c4c4a8f | |
|---|---|---|
| Author: | Basava | |
| Committer: | Basava | |
[BACKPORT 2025.2][#32353] DocDB: Proactively mark leader blacklisted tservers as followers in MetaCache Summary: YBA leader blacklists tservers before restarting each tserver in the process of a rolling restart. Though the leaders have moved away from this node, the other tservers operating on a stale meta-cache view would still try hitting the down tserver for reads/writes if they haven't already noticed that the leader has been moved away. It isn't much of a problem in the usual case since the node might RST the packet and the tserver would move on to try the other replicas before going to the master to re-fetch the tablet locations. But in case of an ungraceful termination/shutdown of the node (or in a kubernetes environment), the connection could keep blackholing and be kept alive for `rpc_connection_timeout_ms` (defaults to 15s). This might affect op latencies which is expected to some extent in an ungraceful shutdown, but isn't really ok on a planned one. Since the latencies get affected for even a planned shutdown in a kubernetes env, it is better it the other tservers proactively mark the tserver to go down as a follower and not route read/write traffic to it (It could still route reads if follower reads is on though). Changes 1. This revision addresses the above issue by propagating the leader blacklisted tservers info on the master -> tserver heartbeat response once it sees the leader count has dropped to 0 (for the blacklisted tsevrer). Tservers mark these leader blacklisted tservers (with 0 leaders) as followers and hence wouldn't route read/write requests to them. When the leader blacklisted tserver gets un-blacklisted though, we don't explicitly reset this info and let the meta-cache figure it out eventually when a follower peer sends the latest consensus info or when the meta-cache ends up going to the master. This propagation is done at most once per tserver per leader blacklist addition. 2. Incase the leader load on the leader blacklisted tservers is 0 while responding to `GetLoadMoveCompletionPercent`, delay until master has seen a heartbeat from all live tservers. This is being done as best effort for the other live tservers to mark the leader blacklisted tserver as follower. Both the above changes are protected under new gflags which default to true (optimizations enabled by default). **Upgrade/Downgrade safety** Added new `leader_blacklisted_tservers_with_no_leaders` to the tserver-master heartbeat message. The tserver checks for the size of the field before trying to access the repeated field, so the opeartion is safe ic case of upgrades/downgrades/ and mixed mode operations. This filed is purely used for an optimization as mentioned above. Note: We could also do the same for blacklisted tservers, and execute `MetaCache::MarkTSFailed`. This would be useful on cluster scale in/scale out activities where requests are being sent to followers (which is rare, and hence covering just the leader case here). Original commit: 5292a99c4417137a2b306ccd65447701ec301168 / D54742 Test Plan: Jenkins Manually tested the following scenario on a local rf3 cluster: 1. `create table test(k int) split into 10 tablets;` 2. set vmodule on `tablet_rpc=1` 3. randomly execut eleader stepdown using `yb-admin --master_addresses=127.0.0.1:7100,127.0.0.2:7100.127.0.0.3:7100 leader_stepdown <tablet>` 4. `insert into test select i from generate_series(1, 10) as i;` and notice the following logs on the tserver hosting the connection ``` I0618 23:14:27.320129 1333856 tablet_rpc.cc:312] vlog1: Not the leader for Write(tablet: 6f37f80451b14092b7af7cee4efa39b5, num_ops: 4, num_attempts: 1, txn: 152ea27d-b9a9-4f0c-9371-87b3d7ca5fc8, subtxn: [none]) retrying with a different replica I0618 23:14:27.322331 1334100 tablet_rpc.cc:312] vlog1: Not the leader for Write(tablet: 5cb9fe88fa8a4ff69f7effa2e8aabcaa, num_ops: 2, num_attempts: 1, txn: 152ea27d-b9a9-4f0c-9371-87b3d7ca5fc8, subtxn: [none]) retrying with a different replica ``` 5. now blacklist one of the tservers using `yb-admin --master_addresses=127.0.0.1:7100,127.0.0.2:7100.127.0.0.3:7100 change_leader_blacklist ADD <ip>:9100` 6. `insert into test select i from generate_series(1, 10) as i;` and notice we don't see any complaints of `Not the leader for Write` (assuming the write is issue after 1s, that is, after the tserver processes the master heartbeat resp). Reviewers: amitanand, neera.mital, mlillibridge Reviewed By: amitanand Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D55077
| Commit: | 8267175 | |
|---|---|---|
| Author: | Timur Yusupov | |
| Committer: | Timur Yusupov | |
[BACKPORT 2025.1][#31930,#31241] docdb: Fix RBS vs ConfigChange races Summary: There is a possibility of RBS vs ConfigChange races, for example: 1. Raft config for the tablet has nodes A (leader), B, C. 2. D is added to Raft config, CHANGE_CONFIG operation is committed and applied on A, B, C 3. RBS A -> D started 4. D is removed from Raft config, CHANGE_CONFIG operation is committed and applied on leader 5. RBS A -> D downloads WAL and completed, D has the latest committed Raft config **Expected result:** orphaned tablet replica on D should be deleted. **Actual result:** we have an orphaned tablet replica (lagging follower) on D that is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader. In order to fix that, behaviour inside `MasterHeartbeatServiceImpl::ProcessTabletReport` is changed to delete a tablet replica which is no longer part of the committed Raft config and Raft config that added that replica is no longer pending (either committed or aborted). The latter condition avoids deleting the newly RBSed tablet replica that is being added to the tablet Raft group. The following logic is implemented to support that: 1. Once tablet leader decides to start RBS replica on another tserver, it will include the current pending Raft config op id (both term and index) into `StartRemoteBootstrapRequestPB` or empty op id when no config change is pending. 2. The bootstrapping replica persists this op id in its consensus metadata as `pending_config_op_id_from_rbs`. It is cleared once the replica's last committed op id either advances to a higher term, or its index reaches/passes the stored pending op id's index - i.e. once the original `CHANGE_CONFIG` operation can no longer be pending (it has either committed or been aborted). 3. `ReportedTabletPB::pending_config_op_id` is added to tserver->master heartbeats. Its value is whichever is set: the replica's currently active pending config op id (a config change in progress on the replica itself) or the pending_config_op_id_from_rbs from (2). 4. When master leader receives a tablet report from a replica that is *not* in the committed Raft config last known by the master, the master triggers `DeleteTabletRequestPB` to that replica if both: a. The reported committed Raft config op id index is <= the committed Raft config op id index last known by master for this tablet. b. Either the report carries no pending op id (empty/missing), or the master can prove it is no longer pending: if the pending op id's term is strictly less than the `current_term` in the master's last-known committed consensus state for this tablet, that `CHANGE_CONFIG` must have already been aborted or committed, and condition (4.a) alone is sufficient. 5. TServer will include tablet in next heartbeat in case DeleteTablet failed due to stale cas_config_opid_index_less_or_equal (this scenario is covered by TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart). Also update master-side of CloneTablet operation to seed the target tablet's committed_consensus_state peers on the master from the source tablet's config so that when the cloned replicas heartbeat back, `ProcessTabletReportBatch` sees them as part of the expected Raft config and does not tombstone them. Added several unit-tests for other RBS vs ConfigChange race scenarios. They are also fixed by the implemented change. Also renamed `RaftConfigPB.opid_index` to `committed_op_index` in order to reflect the actual purpose of this field. **Upgrade/Rollback safety:** New logic at master-side is gated by a new `use_tablet_report_pending_config_op_id` auto flag. Until the whole cluster is fully upgraded, master won't rely on newly added `ReportedTabletPB::pending_config_op_id` field. Original commit: 54c3d4ca73c593b364abf3f9462e788d32a20f53 / D52759 Test Plan: Run the following tests for asan/tsan/debug/relelase for 20 iterations each: - TabletSplitITest.SplitWithParentTabletRbsFromFollower - TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRace - TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart - TabletReplacementITest.TombstoneEvictedReplicaAfterAbortedAddServer - covers https://github.com/yugabyte/yugabyte-db/issues/31241 - TabletReplacementITest.DontDeleteNewReplicaInPendingConfig - TabletReplacementITest.DontDeleteNewReplicaInPendingConfigAfterRbsFromFollowerRf5 Reviewers: zdrudi Reviewed By: zdrudi Subscribers: ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D55088
| Commit: | 1bb39da | |
|---|---|---|
| Author: | Timur Yusupov | |
| Committer: | Timur Yusupov | |
[BACKPORT 2024.2][#31930,#31241] docdb: Fix RBS vs ConfigChange races Summary: There is a possibility of RBS vs ConfigChange races, for example: 1. Raft config for the tablet has nodes A (leader), B, C. 2. D is added to Raft config, CHANGE_CONFIG operation is committed and applied on A, B, C 3. RBS A -> D started 4. D is removed from Raft config, CHANGE_CONFIG operation is committed and applied on leader 5. RBS A -> D downloads WAL and completed, D has the latest committed Raft config **Expected result:** orphaned tablet replica on D should be deleted. **Actual result:** we have an orphaned tablet replica (lagging follower) on D that is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader. In order to fix that, behaviour inside `MasterHeartbeatServiceImpl::ProcessTabletReport` is changed to delete a tablet replica which is no longer part of the committed Raft config and Raft config that added that replica is no longer pending (either committed or aborted). The latter condition avoids deleting the newly RBSed tablet replica that is being added to the tablet Raft group. The following logic is implemented to support that: 1. Once tablet leader decides to start RBS replica on another tserver, it will include the current pending Raft config op id (both term and index) into `StartRemoteBootstrapRequestPB` or empty op id when no config change is pending. 2. The bootstrapping replica persists this op id in its consensus metadata as `pending_config_op_id_from_rbs`. It is cleared once the replica's last committed op id either advances to a higher term, or its index reaches/passes the stored pending op id's index - i.e. once the original `CHANGE_CONFIG` operation can no longer be pending (it has either committed or been aborted). 3. `ReportedTabletPB::pending_config_op_id` is added to tserver->master heartbeats. Its value is whichever is set: the replica's currently active pending config op id (a config change in progress on the replica itself) or the pending_config_op_id_from_rbs from (2). 4. When master leader receives a tablet report from a replica that is *not* in the committed Raft config last known by the master, the master triggers `DeleteTabletRequestPB` to that replica if both: a. The reported committed Raft config op id index is <= the committed Raft config op id index last known by master for this tablet. b. Either the report carries no pending op id (empty/missing), or the master can prove it is no longer pending: if the pending op id's term is strictly less than the `current_term` in the master's last-known committed consensus state for this tablet, that `CHANGE_CONFIG` must have already been aborted or committed, and condition (4.a) alone is sufficient. 5. TServer will include tablet in next heartbeat in case DeleteTablet failed due to stale cas_config_opid_index_less_or_equal (this scenario is covered by TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart). Also update master-side of CloneTablet operation to seed the target tablet's committed_consensus_state peers on the master from the source tablet's config so that when the cloned replicas heartbeat back, `ProcessTabletReportBatch` sees them as part of the expected Raft config and does not tombstone them. Added several unit-tests for other RBS vs ConfigChange race scenarios. They are also fixed by the implemented change. Also renamed `RaftConfigPB.opid_index` to `committed_op_index` in order to reflect the actual purpose of this field. **Upgrade/Rollback safety:** New logic at master-side is gated by a new `use_tablet_report_pending_config_op_id` auto flag. Until the whole cluster is fully upgraded, master won't rely on newly added `ReportedTabletPB::pending_config_op_id` field. Original commit: 54c3d4ca73c593b364abf3f9462e788d32a20f53 / D52759 Test Plan: Run the following tests for asan/tsan/debug/relelase for 20 iterations each: - TabletSplitITest.SplitWithParentTabletRbsFromFollower - TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRace - TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart - TabletReplacementITest.TombstoneEvictedReplicaAfterAbortedAddServer - covers https://github.com/yugabyte/yugabyte-db/issues/31241 - TabletReplacementITest.DontDeleteNewReplicaInPendingConfig - TabletReplacementITest.DontDeleteNewReplicaInPendingConfigAfterRbsFromFollowerRf5 Reviewers: zdrudi Reviewed By: zdrudi Subscribers: ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D55089
| Commit: | f3a664a | |
|---|---|---|
| Author: | Timur Yusupov | |
| Committer: | Timur Yusupov | |
[BACKPORT 2024.2.10][#31930,#31241] docdb: Fix RBS vs ConfigChange races Summary: There is a possibility of RBS vs ConfigChange races, for example: 1. Raft config for the tablet has nodes A (leader), B, C. 2. D is added to Raft config, CHANGE_CONFIG operation is committed and applied on A, B, C 3. RBS A -> D started 4. D is removed from Raft config, CHANGE_CONFIG operation is committed and applied on leader 5. RBS A -> D downloads WAL and completed, D has the latest committed Raft config **Expected result:** orphaned tablet replica on D should be deleted. **Actual result:** we have an orphaned tablet replica (lagging follower) on D that is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader. In order to fix that, behaviour inside `MasterHeartbeatServiceImpl::ProcessTabletReport` is changed to delete a tablet replica which is no longer part of the committed Raft config and Raft config that added that replica is no longer pending (either committed or aborted). The latter condition avoids deleting the newly RBSed tablet replica that is being added to the tablet Raft group. The following logic is implemented to support that: 1. Once tablet leader decides to start RBS replica on another tserver, it will include the current pending Raft config op id (both term and index) into `StartRemoteBootstrapRequestPB` or empty op id when no config change is pending. 2. The bootstrapping replica persists this op id in its consensus metadata as `pending_config_op_id_from_rbs`. It is cleared once the replica's last committed op id either advances to a higher term, or its index reaches/passes the stored pending op id's index - i.e. once the original `CHANGE_CONFIG` operation can no longer be pending (it has either committed or been aborted). 3. `ReportedTabletPB::pending_config_op_id` is added to tserver->master heartbeats. Its value is whichever is set: the replica's currently active pending config op id (a config change in progress on the replica itself) or the pending_config_op_id_from_rbs from (2). 4. When master leader receives a tablet report from a replica that is *not* in the committed Raft config last known by the master, the master triggers `DeleteTabletRequestPB` to that replica if both: a. The reported committed Raft config op id index is <= the committed Raft config op id index last known by master for this tablet. b. Either the report carries no pending op id (empty/missing), or the master can prove it is no longer pending: if the pending op id's term is strictly less than the `current_term` in the master's last-known committed consensus state for this tablet, that `CHANGE_CONFIG` must have already been aborted or committed, and condition (4.a) alone is sufficient. 5. TServer will include tablet in next heartbeat in case DeleteTablet failed due to stale cas_config_opid_index_less_or_equal (this scenario is covered by TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart). Also update master-side of CloneTablet operation to seed the target tablet's committed_consensus_state peers on the master from the source tablet's config so that when the cloned replicas heartbeat back, `ProcessTabletReportBatch` sees them as part of the expected Raft config and does not tombstone them. Added several unit-tests for other RBS vs ConfigChange race scenarios. They are also fixed by the implemented change. Also renamed `RaftConfigPB.opid_index` to `committed_op_index` in order to reflect the actual purpose of this field. **Upgrade/Rollback safety:** New logic at master-side is gated by a new `use_tablet_report_pending_config_op_id` auto flag. Until the whole cluster is fully upgraded, master won't rely on newly added `ReportedTabletPB::pending_config_op_id` field. Original commit: 54c3d4ca73c593b364abf3f9462e788d32a20f53 / D52759 Test Plan: Run the following tests for asan/tsan/debug/relelase for 20 iterations each: - TabletSplitITest.SplitWithParentTabletRbsFromFollower - TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRace - TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart - TabletReplacementITest.TombstoneEvictedReplicaAfterAbortedAddServer - covers https://github.com/yugabyte/yugabyte-db/issues/31241 - TabletReplacementITest.DontDeleteNewReplicaInPendingConfig - TabletReplacementITest.DontDeleteNewReplicaInPendingConfigAfterRbsFromFollowerRf5 Reviewers: zdrudi Reviewed By: zdrudi Subscribers: ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D55166
| Commit: | 4707ce3 | |
|---|---|---|
| Author: | Timur Yusupov | |
| Committer: | Timur Yusupov | |
[BACKPORT 2024.2.10][#27056] docdb: Fixed tablet split vs RBS from the follower race Summary: There is a possibility of tablet split vs RBS from follower race: 1. Parent tablet leader peers A-C accept a SPLIT_OP (op_id: 1.4). Leader (A) and 1st follower (B) apply SPLIT_OP, 2nd follower (C) doesn't apply it yet. 2. Parent tablet leader (node A) accepts CHANGE_CONFIG_OP (op_id: 1.5) to add a fourth peer (D) but doesn't apply it yet. 3. Parent tablet 2nd follower (C) still hasn't yet applied SPLIT_OP (1.4). 4. RBS for parent tablet peer (D) starts from the follower (C) and tablet metadata (tablet_data_state == TABLET_DATA_READY) is downloaded. 5. Parent tablet peers A-C completed applying the SPLIT_OP (1.4), child tablets have Raft config with 3 peers. 6. Parent tablet peers A-C apply CHANGE_CONFIG_OP (1.5) and now have committed Raft config with 4 peers. 7. Parent tablet peer D does local bootstrap and replays SPLIT_OP (1.4) as part of bootstrap. Due to tablet_data_state is TABLET_DATA_READY but not TABLET_DATA_SPLIT_COMPLETED replay does SPLIT_OP apply and creates child tablet peer. After that, 4th child tablet peer (D) is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader. This change fixes this race by rejecting RBS from the follower that is in progress of applying SPLIT_OP and RBS attempt will be retried later. Original commit: 78af3a208a6bfc005eedc0ed6e410d22f4d24758 / D48853 **Upgrade/Rollback safety:** New error code will be printed by old nodes as just number in case of RBS failure during upgrade but this is safe. Test Plan: TabletSplitITest.SplitWithParentTabletRbsFromFollower, TabletSplitITest.SplitWithParentTabletMove, RemoteBootstrapsFromNodeWithUncommittedSplitOp - 30 runs per each of asan/tsan/debug/release builds Reviewers: arybochkin Reviewed By: arybochkin Subscribers: ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D55142
| Commit: | ccac6fb | |
|---|---|---|
| Author: | Basava | |
| Committer: | Basava | |
[BACKPORT 2026.1][#32353] DocDB: Proactively mark leader blacklisted tservers as followers in MetaCache Summary: YBA leader blacklists tservers before restarting each tserver in the process of a rolling restart. Though the leaders have moved away from this node, the other tservers operating on a stale meta-cache view would still try hitting the down tserver for reads/writes if they haven't already noticed that the leader has been moved away. It isn't much of a problem in the usual case since the node might RST the packet and the tserver would move on to try the other replicas before going to the master to re-fetch the tablet locations. But in case of an ungraceful termination/shutdown of the node (or in a kubernetes environment), the connection could keep blackholing and be kept alive for `rpc_connection_timeout_ms` (defaults to 15s). This might affect op latencies which is expected to some extent in an ungraceful shutdown, but isn't really ok on a planned one. Since the latencies get affected for even a planned shutdown in a kubernetes env, it is better it the other tservers proactively mark the tserver to go down as a follower and not route read/write traffic to it (It could still route reads if follower reads is on though). Changes 1. This revision addresses the above issue by propagating the leader blacklisted tservers info on the master -> tserver heartbeat response once it sees the leader count has dropped to 0 (for the blacklisted tsevrer). Tservers mark these leader blacklisted tservers (with 0 leaders) as followers and hence wouldn't route read/write requests to them. When the leader blacklisted tserver gets un-blacklisted though, we don't explicitly reset this info and let the meta-cache figure it out eventually when a follower peer sends the latest consensus info or when the meta-cache ends up going to the master. This propagation is done at most once per tserver per leader blacklist addition. 2. Incase the leader load on the leader blacklisted tservers is 0 while responding to `GetLoadMoveCompletionPercent`, delay until master has seen a heartbeat from all live tservers. This is being done as best effort for the other live tservers to mark the leader blacklisted tserver as follower. Both the above changes are protected under new gflags which default to true (optimizations enabled by default). **Upgrade/Downgrade safety** Added new `leader_blacklisted_tservers_with_no_leaders` to the tserver-master heartbeat message. The tserver checks for the size of the field before trying to access the repeated field, so the opeartion is safe ic case of upgrades/downgrades/ and mixed mode operations. This filed is purely used for an optimization as mentioned above. Note: We could also do the same for blacklisted tservers, and execute `MetaCache::MarkTSFailed`. This would be useful on cluster scale in/scale out activities where requests are being sent to followers (which is rare, and hence covering just the leader case here). Original commit: 5292a99c4417137a2b306ccd65447701ec301168 / D54742 Test Plan: Jenkins Manually tested the following scenario on a local rf3 cluster: 1. `create table test(k int) split into 10 tablets;` 2. set vmodule on `tablet_rpc=1` 3. randomly execut eleader stepdown using `yb-admin --master_addresses=127.0.0.1:7100,127.0.0.2:7100.127.0.0.3:7100 leader_stepdown <tablet>` 4. `insert into test select i from generate_series(1, 10) as i;` and notice the following logs on the tserver hosting the connection ``` I0618 23:14:27.320129 1333856 tablet_rpc.cc:312] vlog1: Not the leader for Write(tablet: 6f37f80451b14092b7af7cee4efa39b5, num_ops: 4, num_attempts: 1, txn: 152ea27d-b9a9-4f0c-9371-87b3d7ca5fc8, subtxn: [none]) retrying with a different replica I0618 23:14:27.322331 1334100 tablet_rpc.cc:312] vlog1: Not the leader for Write(tablet: 5cb9fe88fa8a4ff69f7effa2e8aabcaa, num_ops: 2, num_attempts: 1, txn: 152ea27d-b9a9-4f0c-9371-87b3d7ca5fc8, subtxn: [none]) retrying with a different replica ``` 5. now blacklist one of the tservers using `yb-admin --master_addresses=127.0.0.1:7100,127.0.0.2:7100.127.0.0.3:7100 change_leader_blacklist ADD <ip>:9100` 6. `insert into test select i from generate_series(1, 10) as i;` and notice we don't see any complaints of `Not the leader for Write` (assuming the write is issue after 1s, that is, after the tserver processes the master heartbeat resp). Reviewers: amitanand, neera.mital, mlillibridge Reviewed By: amitanand Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D55076
| Commit: | c691cb2 | |
|---|---|---|
| Author: | Craig Soules | |
| Committer: | Sanketh I | |
[BACKPORT 2025.2][#30578] YSQL: Reset auto-analyze mutation counts after a manual ANALYZE Summary: Reset auto-analyze mutation counters after user-initiated ANALYZE Original commit: b6bbc7a663df2252865be498931bd4484ac7c224 / #31849 Cherry-picked from: 62358fae62da8e85c60a8dfb9c27e6e0275668b8 (2026.1 backport) Backport conflicts resolved (2026.1 -> 2025.2): - src/yb/ash/wait_state.h: kept only the new kResetAutoAnalyzeMutationCounters enumerator; the adjacent kGetTabletForKey / kRemotePgExec entries are 2026.1-only features not present in 2025.2. - src/yb/yql/pggate/ybc_pggate.h: kept only the YBCResetAutoAnalyzeMutationCounters declaration; the adjacent PgGlobalViewRead API block is a 2026.1-only feature. - src/yb/tserver/pg_client_service.cc: added the ResetAutoAnalyzeMutationCounters handler; dropped the bundled RemoteExec handler, which is a 2026.1-only feature absent from 2025.2. - src/yb/tserver/stateful_services/pg_auto_analyze_service.cc: applied the UpdateTableMutationsAfterAnalyze refactor to call SubtractPgAutoAnalyzeMutationCounts; the conflict was caused by 2025.2 using explicit PB types and a NewWriteOp overload without session->arena(). The table_tuple_count_ erase is preserved in the new snapshot-building loop. User-initiated ANALYZE does not reset the auto-analyze service's accumulated mutation count. The next periodic tick can therefore trigger an auto-analyze on a table the user just analyzed, wasting work. Add ResetAutoAnalyzeMutationCounters RPC (pg_client.proto / pg_client_service) called from do_analyze_rel after a full ANALYZE. The RPC resets the YCQL service table mutations to 0 for the analyzed table. Reset is gated to match PostgreSQL's semantics for updating its changes_since_analyze counters (fires only when no column list given) and is suppressed for auto-analyze service's internal connections and other internal backends. Auto analyze continues to reset mutations using the existing logic to subtract mutations on its side. Mutations are stored in a separate YCQL table so the mutations update is not transactional. A failed manual ANALYZE can still reset counters to 0. Fixing this is tracked in #32081. Extract mutation-update logic into helper functions in pg_auto_analyze_table that use conditional YCQL writes for better reuse. - ResetPgAutoAnalyzeMutationCounts: sets mutations to 0 (IF EXISTS) - SubtractPgAutoAnalyzeMutationCounts: subtracts snapshot mutations with clamping. Emits two conditional writes per table — one sets to 0 if current < snapshot, another subtracts if current >= snapshot. This prevents the count from going negative when a manual-ANALYZE reset races with auto-analyze post-ANALYZE mutation subtraction. UpdateTableMutationsAfterAnalyze refactored to use SubtractPgAutoAnalyzeMutationCounts instead of building operations directly. New RPC: PgClientService.ResetAutoAnalyzeMutationCounters (pg_client.proto). Additive only — no existing field numbers or messages altered. Upgrade/rollback safety: New RPC is between PG and local tserver so no upgrade/rollback issues. Test Plan: - PgAutoAnalyzeTest.ManualAnalyzeResetsMutationCount: covers ANALYZE, ANALYZE(col), VACUUM ANALYZE, VACUUM ANALYZE(col), asserting reset only when no column list provided. - PgAutoAnalyzeTest.InternalAnalyzeDoesNotResetMutationCount: confirms internal connections do not trigger reset. - PgAutoAnalyzeTest.ManualAnalyzePartitionedTableResetsPartitionMutationCounts: exercises the partitioned-table path. - Existing PgAutoAnalyzeTest cases continue to pass. Reviewers: kfranz, pjain Reviewed By: kfranz Differential Revision: https://phorge.dev.yugabyte.com/D55092
| Commit: | 3d1b251 | |
|---|---|---|
| Author: | Timur Yusupov | |
| Committer: | Timur Yusupov | |
[BACKPORT 2025.2][#31930,#31241] docdb: Fix RBS vs ConfigChange races Summary: There is a possibility of RBS vs ConfigChange races, for example: 1. Raft config for the tablet has nodes A (leader), B, C. 2. D is added to Raft config, CHANGE_CONFIG operation is committed and applied on A, B, C 3. RBS A -> D started 4. D is removed from Raft config, CHANGE_CONFIG operation is committed and applied on leader 5. RBS A -> D downloads WAL and completed, D has the latest committed Raft config **Expected result:** orphaned tablet replica on D should be deleted. **Actual result:** we have an orphaned tablet replica (lagging follower) on D that is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader. In order to fix that, behaviour inside `MasterHeartbeatServiceImpl::ProcessTabletReport` is changed to delete a tablet replica which is no longer part of the committed Raft config and Raft config that added that replica is no longer pending (either committed or aborted). The latter condition avoids deleting the newly RBSed tablet replica that is being added to the tablet Raft group. The following logic is implemented to support that: 1. Once tablet leader decides to start RBS replica on another tserver, it will include the current pending Raft config op id (both term and index) into `StartRemoteBootstrapRequestPB` or empty op id when no config change is pending. 2. The bootstrapping replica persists this op id in its consensus metadata as `pending_config_op_id_from_rbs`. It is cleared once the replica's last committed op id either advances to a higher term, or its index reaches/passes the stored pending op id's index - i.e. once the original `CHANGE_CONFIG` operation can no longer be pending (it has either committed or been aborted). 3. `ReportedTabletPB::pending_config_op_id` is added to tserver->master heartbeats. Its value is whichever is set: the replica's currently active pending config op id (a config change in progress on the replica itself) or the pending_config_op_id_from_rbs from (2). 4. When master leader receives a tablet report from a replica that is *not* in the committed Raft config last known by the master, the master triggers `DeleteTabletRequestPB` to that replica if both: a. The reported committed Raft config op id index is <= the committed Raft config op id index last known by master for this tablet. b. Either the report carries no pending op id (empty/missing), or the master can prove it is no longer pending: if the pending op id's term is strictly less than the `current_term` in the master's last-known committed consensus state for this tablet, that `CHANGE_CONFIG` must have already been aborted or committed, and condition (4.a) alone is sufficient. 5. TServer will include tablet in next heartbeat in case DeleteTablet failed due to stale cas_config_opid_index_less_or_equal (this scenario is covered by TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart). Also update master-side of CloneTablet operation to seed the target tablet's committed_consensus_state peers on the master from the source tablet's config so that when the cloned replicas heartbeat back, `ProcessTabletReportBatch` sees them as part of the expected Raft config and does not tombstone them. Added several unit-tests for other RBS vs ConfigChange race scenarios. They are also fixed by the implemented change. Also renamed `RaftConfigPB.opid_index` to `committed_op_index` in order to reflect the actual purpose of this field. **Upgrade/Rollback safety:** New logic at master-side is gated by a new `use_tablet_report_pending_config_op_id` auto flag. Until the whole cluster is fully upgraded, master won't rely on newly added `ReportedTabletPB::pending_config_op_id` field. Original commit: 54c3d4ca73c593b364abf3f9462e788d32a20f53 / D52759 Test Plan: Run the following tests for asan/tsan/debug/relelase for 20 iterations each: - TabletSplitITest.SplitWithParentTabletRbsFromFollower - TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRace - TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart - TabletReplacementITest.TombstoneEvictedReplicaAfterAbortedAddServer - covers https://github.com/yugabyte/yugabyte-db/issues/31241 - TabletReplacementITest.DontDeleteNewReplicaInPendingConfig - TabletReplacementITest.DontDeleteNewReplicaInPendingConfigAfterRbsFromFollowerRf5 Reviewers: zdrudi Reviewed By: zdrudi Subscribers: ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D55040
| Commit: | 6e55c4e | |
|---|---|---|
| Author: | Timur Yusupov | |
| Committer: | Timur Yusupov | |
[BACKPORT 2026.1][#31930,#31241] docdb: Fix RBS vs ConfigChange races Summary: There is a possibility of RBS vs ConfigChange races, for example: 1. Raft config for the tablet has nodes A (leader), B, C. 2. D is added to Raft config, CHANGE_CONFIG operation is committed and applied on A, B, C 3. RBS A -> D started 4. D is removed from Raft config, CHANGE_CONFIG operation is committed and applied on leader 5. RBS A -> D downloads WAL and completed, D has the latest committed Raft config **Expected result:** orphaned tablet replica on D should be deleted. **Actual result:** we have an orphaned tablet replica (lagging follower) on D that is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader. In order to fix that, behaviour inside `MasterHeartbeatServiceImpl::ProcessTabletReport` is changed to delete a tablet replica which is no longer part of the committed Raft config and Raft config that added that replica is no longer pending (either committed or aborted). The latter condition avoids deleting the newly RBSed tablet replica that is being added to the tablet Raft group. The following logic is implemented to support that: 1. Once tablet leader decides to start RBS replica on another tserver, it will include the current pending Raft config op id (both term and index) into `StartRemoteBootstrapRequestPB` or empty op id when no config change is pending. 2. The bootstrapping replica persists this op id in its consensus metadata as `pending_config_op_id_from_rbs`. It is cleared once the replica's last committed op id either advances to a higher term, or its index reaches/passes the stored pending op id's index - i.e. once the original `CHANGE_CONFIG` operation can no longer be pending (it has either committed or been aborted). 3. `ReportedTabletPB::pending_config_op_id` is added to tserver->master heartbeats. Its value is whichever is set: the replica's currently active pending config op id (a config change in progress on the replica itself) or the pending_config_op_id_from_rbs from (2). 4. When master leader receives a tablet report from a replica that is *not* in the committed Raft config last known by the master, the master triggers `DeleteTabletRequestPB` to that replica if both: a. The reported committed Raft config op id index is <= the committed Raft config op id index last known by master for this tablet. b. Either the report carries no pending op id (empty/missing), or the master can prove it is no longer pending: if the pending op id's term is strictly less than the `current_term` in the master's last-known committed consensus state for this tablet, that `CHANGE_CONFIG` must have already been aborted or committed, and condition (4.a) alone is sufficient. 5. TServer will include tablet in next heartbeat in case DeleteTablet failed due to stale cas_config_opid_index_less_or_equal (this scenario is covered by TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart). Also update master-side of CloneTablet operation to seed the target tablet's committed_consensus_state peers on the master from the source tablet's config so that when the cloned replicas heartbeat back, `ProcessTabletReportBatch` sees them as part of the expected Raft config and does not tombstone them. Added several unit-tests for other RBS vs ConfigChange race scenarios. They are also fixed by the implemented change. Also renamed `RaftConfigPB.opid_index` to `committed_op_index` in order to reflect the actual purpose of this field. **Upgrade/Rollback safety:** New logic at master-side is gated by a new `use_tablet_report_pending_config_op_id` auto flag. Until the whole cluster is fully upgraded, master won't rely on newly added `ReportedTabletPB::pending_config_op_id` field. Original commit: 54c3d4ca73c593b364abf3f9462e788d32a20f53 / D52759 Test Plan: Run the following tests for asan/tsan/debug/relelase for 20 iterations each: - TabletSplitITest.SplitWithParentTabletRbsFromFollower - TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRace - TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart - TabletReplacementITest.TombstoneEvictedReplicaAfterAbortedAddServer - covers https://github.com/yugabyte/yugabyte-db/issues/31241 - TabletReplacementITest.DontDeleteNewReplicaInPendingConfig - TabletReplacementITest.DontDeleteNewReplicaInPendingConfigAfterRbsFromFollowerRf5 Reviewers: zdrudi Reviewed By: zdrudi Subscribers: ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D54974
| Commit: | 469dc36 | |
|---|---|---|
| Author: | Sumukh-Phalgaonkar | |
| Committer: | Sumukh-Phalgaonkar | |
[BACKPORT 2025.2][#32116] CDC: Add support to create table bound gRPC streams Summary: ### Backport Description Minor merge conflicts in `src/yb/master/xrepl_catalog_manager.cc`, due to a parameter not being present in the function `IsTableEligibleForCDCSDKStream()`. ### Original Description ##### Code changes summary Currently when a replication slot is created, retention barriers are setup and cdc_state table entries are written for all the tablets in the DB. In an environment where large number of tables are present and only a small subset is being used for CDC, this leads to unnecessary retention barrier setup. Also the cdc_state table is bloated with unnecessary entries. To prevent this, this diff introduces a mechanism to create gRPC streams that are bound to only specific tables at the time of their creation. To create such a stream, a comma separated list of table ids should be provided to the create_change_data_stream yb-admin command. The syntax is as follows: ``` ./yb-admin create_change_data_stream ysql.<DB-name> EXPLICIT CHANGE NOEXPORT_SNAPSHOT DYNAMIC_TABLES_DISABLED <comma separated table_ids> ``` For example: ``` ./yb-admin create_change_data_stream ysql.yugabyte EXPLICIT CHANGE NOEXPORT_SNAPSHOT DYNAMIC_TABLES_DISABLED 000034e1000030008000000000004000,000034e1000030008000000000004005 CDC Stream ID: 9fbec9b0395a2caacd48676d714ced0d ``` The table_ids are passed to the `CreateCDCStream` rpc by populating the `bound_table_ids` field in the `CDCSDKStreamCreateOptionsPB`. Only these table_ids are written to the stream metadata. The retention barriers are set on the tablets of only these tables, and their entries are written to the cdc_state table. Dynamic table addition is disabled for the table bound streams, meaning that the tables which can be polled using these streams is fixed at the stream creation. Any attmept to create such streams with dynamic tables enabled will fail. Also such streams can only be created for gRPC model. ##### Upgrade / Rollback safety Only proto change made in this diff is in `CDCSDKStreamCreateOptionsPB` which is a part of `CreateCDCStreamRequestPB`. The CreateCDCStream rpc flows from the tserver to the master. Since all the masters are upgraded before the tservers, this change is upgrade safe. Additionally the bound_table_ids field added in `CDCSDKStreamCreateOptionsPB` is an optional field. To make the repeated field optional it has been wrapped in a separate proto called `CDCSDKBoundTableIds`. A table bound stream created before rollback will continue to operate as intended after rollback, i.e post rollback user can use the table bound stream to get the change events from the tables present in the stream metadata. Hence this change is rollback safe. ##### Considerations for colocated tables If a stream is created such that it is bound to subset of colocated tables residing on the tablet, cdcsdk_producer will filter out the change records corresponding to other tables. ##### Considerations for connector NA Original commit: 6d1ef2a86b3e1b0e3055ba1614944245c88c479e / D54632 Test Plan: New tests added: - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestgRPCStreamBoundToSpecificTables' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestgRPCStreamBoundToSpecificColocatedTables' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestTableBoundStreamRejectsWithReplicationSlot' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestTableBoundStreamDisablesDynamicAddition' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestCreationOfgRPCStreamBoundToSpecificTablesViaYBAdmin' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestTableBoundStreamYbAdminRejectsTableFromDifferentNamespace' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestTableBoundStreamYbAdminRejectsTableFromDifferentNamespaceTestTableBoundStreamYbAdminRejectsDynamicTablesEnabled' Reviewers: #db-approvers, xCluster, hsunder, skumar, asrinivasan, devansh.singhal Reviewed By: devansh.singhal Subscribers: svc_phabricator, ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D55015
| Commit: | 5292a99 | |
|---|---|---|
| Author: | Basava | |
| Committer: | Basava | |
[#32353] DocDB: Proactively mark leader blacklisted tservers as followers in MetaCache Summary: YBA leader blacklists tservers before restarting each tserver in the process of a rolling restart. Though the leaders have moved away from this node, the other tservers operating on a stale meta-cache view would still try hitting the down tserver for reads/writes if they haven't already noticed that the leader has been moved away. It isn't much of a problem in the usual case since the node might RST the packet and the tserver would move on to try the other replicas before going to the master to re-fetch the tablet locations. But in case of an ungraceful termination/shutdown of the node (or in a kubernetes environment), the connection could keep blackholing and be kept alive for `rpc_connection_timeout_ms` (defaults to 15s). This might affect op latencies which is expected to some extent in an ungraceful shutdown, but isn't really ok on a planned one. Since the latencies get affected for even a planned shutdown in a kubernetes env, it is better it the other tservers proactively mark the tserver to go down as a follower and not route read/write traffic to it (It could still route reads if follower reads is on though). Changes 1. This revision addresses the above issue by propagating the leader blacklisted tservers info on the master -> tserver heartbeat response once it sees the leader count has dropped to 0 (for the blacklisted tsevrer). Tservers mark these leader blacklisted tservers (with 0 leaders) as followers and hence wouldn't route read/write requests to them. When the leader blacklisted tserver gets un-blacklisted though, we don't explicitly reset this info and let the meta-cache figure it out eventually when a follower peer sends the latest consensus info or when the meta-cache ends up going to the master. This propagation is done at most once per tserver per leader blacklist addition. 2. Incase the leader load on the leader blacklisted tservers is 0 while responding to `GetLoadMoveCompletionPercent`, delay until master has seen a heartbeat from all live tservers. This is being done as best effort for the other live tservers to mark the leader blacklisted tserver as follower. Both the above changes are protected under new gflags which default to true (optimizations enabled by default). **Upgrade/Downgrade safety** Added new `leader_blacklisted_tservers_with_no_leaders` to the tserver-master heartbeat message. The tserver checks for the size of the field before trying to access the repeated field, so the opeartion is safe ic case of upgrades/downgrades/ and mixed mode operations. This filed is purely used for an optimization as mentioned above. Note: We could also do the same for blacklisted tservers, and execute `MetaCache::MarkTSFailed`. This would be useful on cluster scale in/scale out activities where requests are being sent to followers (which is rare, and hence covering just the leader case here). Test Plan: Jenkins Manually tested the following scenario on a local rf3 cluster: 1. `create table test(k int) split into 10 tablets;` 2. set vmodule on `tablet_rpc=1` 3. randomly execut eleader stepdown using `yb-admin --master_addresses=127.0.0.1:7100,127.0.0.2:7100.127.0.0.3:7100 leader_stepdown <tablet>` 4. `insert into test select i from generate_series(1, 10) as i;` and notice the following logs on the tserver hosting the connection ``` I0618 23:14:27.320129 1333856 tablet_rpc.cc:312] vlog1: Not the leader for Write(tablet: 6f37f80451b14092b7af7cee4efa39b5, num_ops: 4, num_attempts: 1, txn: 152ea27d-b9a9-4f0c-9371-87b3d7ca5fc8, subtxn: [none]) retrying with a different replica I0618 23:14:27.322331 1334100 tablet_rpc.cc:312] vlog1: Not the leader for Write(tablet: 5cb9fe88fa8a4ff69f7effa2e8aabcaa, num_ops: 2, num_attempts: 1, txn: 152ea27d-b9a9-4f0c-9371-87b3d7ca5fc8, subtxn: [none]) retrying with a different replica ``` 5. now blacklist one of the tservers using `yb-admin --master_addresses=127.0.0.1:7100,127.0.0.2:7100.127.0.0.3:7100 change_leader_blacklist ADD <ip>:9100` 6. `insert into test select i from generate_series(1, 10) as i;` and notice we don't see any complaints of `Not the leader for Write` (assuming the write is issue after 1s, that is, after the tserver processes the master heartbeat resp). Reviewers: amitanand, neera.mital, mlillibridge Reviewed By: amitanand Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D54742
| Commit: | 3d93263 | |
|---|---|---|
| Author: | Sergei Politov | |
| Committer: | Sergei Politov | |
[BACKPORT 2025.2][#30883] DocDB: Show vector index space usage in yb-master and yb-tserver UI pages Summary: The on-disk size breakdown shown on the yb-tserver and yb-master web UI pages did not account for vector indexes, so a table with a vector index under-reported its actual disk footprint. Add VectorLSM::OnDiskSize, which sums the sizes of the immutable chunk files currently on disk, and expose it through DocVectorIndex::OnDiskSize and VectorIndexList::OnDiskSize. TabletPeer aggregates the per-tablet vector index size into a new TabletOnDiskSizeInfo::vector_index_disk_size field, which is folded into active_on_disk_size and serialized in TabletStatusPB. Propagate the size to the master via a new vector_index_size field in TabletDriveStorageMetadataPB and TabletReplicaDriveInfo. The tserver tables/tablets pages and the master tables page now render a "Vector Indexes" line in the size breakdown, and the corresponding JSON endpoints expose vector_index_size. The HTML line is rendered only when the size is non-zero, so tables without a vector index are not cluttered with "Vector Indexes: 0B". Also make TabletVectorIndexes::List, TabletVectorIndexes::Collect, and TabletComponent::VectorIndexesList return the VectorIndexList wrapper instead of the raw docdb::DocVectorIndexesPtr. This removes the repeated VectorIndexList(...) wrapping at call sites; the few places that need the underlying pointer for the docdb write/apply path use the new VectorIndexList::impl accessor. --- **Upgrade / Rollback safety:** Adds field used by new functionality. The old code would just ignore it. New code would not show usage if information is received from node with an old code. --- _automated · Claude Code (Opus 4.8)_ Original commit: 3b4a0bafbf0850cbc7ae982c0cebf89fcca6021a / D54922 Test Plan: ./yb_build.sh debug --cxx-test pg_vector_index-test --gtest_filter 'PgVectorIndexTest.OnDiskSize*' Manually verified on a local cluster: a table with an ybhnsw vector index reports a non-zero "Vector Indexes" size on both the tserver (:9000) and master (:7000) UI pages and in their JSON endpoints, while a table without a vector index reports none. Reviewers: arybochkin Reviewed By: arybochkin Subscribers: ybase, yql Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D55051
| Commit: | 685237b | |
|---|---|---|
| Author: | Abhiramjampani | |
| Committer: | Abhiramjampani | |
[#30553] CDC: support publish action subsets in publications Summary: ###### Code changes summary: We did not support filtering publication DMLs by operation type using PostgreSQL `publish` syntax, for example: `CREATE PUBLICATION pub FOR TABLE t WITH (publish = 'insert')` and `ALTER PUBLICATION pub SET (publish = 'update, delete')`. This change enables that syntax by removing the YB-only restriction that required all publication actions to be enabled together, allowing subset publish options. It also adds `pg_publication` as a new catalog table in the stream metadata (alongside `pg_class` and `pg_publication_rel`), so the virtual WAL can poll it and notice when the publish actions of a publication change via `ALTER PUBLICATION SET (publish = ...)`. On an `UPDATE` to `pg_publication` for a tracked publication, we trigger a publication refresh. To tell a real `ALTER PUBLICATION` apart from the implicit refresh we do when a `CREATE TABLE` gets auto-added to a `FOR ALL TABLES` publication, we added a boolean field `explicit_alter_publication_detected` to `GetConsistentChangesResponsePB`. It's true for `ALTER PUBLICATION` and false for the `CREATE TABLE` auto-add case. The catalog cache reset and `syscache` invalidation callbacks are now fired only when this field is true, so we avoid re-sending `RELATION` messages for tables the consumer already knows about in the `CREATE TABLE` case. ###### Upgrade/rollback safety considerations: The new proto field `explicit_alter_publication_detected ` (field 5 in GetConsistentChangesResponsePB) is an optional bool that defaults to false. This is a local tserver-to-pg RPC message change (CDC consistent changes response), so no AutoFlag is needed per the upgrade safety handbook. During rollback, the field is absent from responses, defaulting to false, which matches the pre-existing behavior before this diff. Adding `pg_publication` to the stream metadata is safe because it is added alongside `pg_class` and `pg_publication_rel` during stream initialization in `xrepl_catalog_manager.cc`. New streams created after the upgrade will include `pg_publication` in their polling list. During rollback, old tserver code will not poll `pg_publication`, so ALTER PUBLICATION SET changes won't be detected this matches the pre-upgrade behavior. Existing streams are unaffected since the table is added at stream creation time. ###### Considerations for colocated tables: This fixes `ALTER PUBLICATION` for colocated tables. Earlier the change was ignored by the stream; now it is applied correctly. ###### Compatibility with logical and gRPC streams: Compatible with logical stream, gRPC streams are not affected. ###### Considerations for connector: N/A Test Plan: ./yb_build.sh debug --java-test org.yb.pgsql.TestPgReplicationSlot#testPublishFilterInsertOnly ./yb_build.sh debug --java-test org.yb.pgsql.TestPgReplicationSlot#testPublishFilterUpdateOnly ./yb_build.sh debug --java-test org.yb.pgsql.TestPgReplicationSlot#testPublishFilterDeleteOnly ./yb_build.sh debug --java-test org.yb.pgsql.TestPgReplicationSlot#testPublishFilterInsertAndUpdate ./yb_build.sh debug --java-test org.yb.pgsql.TestPgReplicationSlot#testPublishFilterUpdateAndDelete ./yb_build.sh debug --java-test org.yb.pgsql.TestPgReplicationSlot#testPublishFilterInsertAndDelete ./yb_build.sh debug --java-test org.yb.pgsql.TestPgReplicationSlot#testAlterPublicationPublishOption Reviewers: sumukh.phalgaonkar, skumar, jason, xCluster, hsunder, stiwary, #db-approvers Reviewed By: sumukh.phalgaonkar, #db-approvers Subscribers: ybase, jason, yql, ycdcxcluster Differential Revision: https://phorge.dev.yugabyte.com/D50894
| Commit: | 2fd1208 | |
|---|---|---|
| Author: | Anton Rybochkin | |
| Committer: | Anton Rybochkin | |
[#31542] docdb: Vector Index: Chunked compaction implementation Summary: The change introduces a chunked compaction. It allows to not build one in-memory merged index for all input vectors but to output into multiple chunk files, each bounded by a configurable memory budget, to reduce OOM risk. If the limit is very small, compaction still produces at least one vector per output chunk. New gflag `vector_index_compaction_chunk_max_mem_store_size_mb` (runtime, default 0) is introduced and should be greater than 0 to enable chunked compaction. **Upgrade/Rollback safety:** Just a comment update in .proto file. Test Plan: ./yb_build.sh --cxx-test ann_methods_vector_lsm-test --gtest_filter VectorLSMTest.ChunkedCompactionRespectsMemStoreLimit/kHnswlib ./yb_build.sh --cxx-test ann_methods_vector_lsm-test --gtest_filter VectorLSMTest.ChunkedCompactionRespectsMemStoreLimit/kUsearch ./yb_build.sh --cxx-test ann_methods_vector_lsm-test --gtest_filter VectorLSMTest.OpenAfterChunkedCompaction/kHnswlib ./yb_build.sh --cxx-test ann_methods_vector_lsm-test --gtest_filter VectorLSMTest.OpenAfterChunkedCompaction/kUsearch ./yb_build.sh --cxx-test ann_methods_vector_lsm-test --gtest_filter VectorLSMTest.DefaultCompactionMergesMultipleChunks/kHnswlib ./yb_build.sh --cxx-test ann_methods_vector_lsm-test --gtest_filter VectorLSMTest.DefaultCompactionMergesMultipleChunks/kUsearch Reviewers: sergei, zdrudi Reviewed By: sergei, zdrudi Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D54283
| Commit: | 3f6e528 | |
|---|---|---|
| Author: | Sergei Politov | |
| Committer: | Sergei Politov | |
[BACKPORT 2026.1][#30883] DocDB: Show vector index space usage in yb-master and yb-tserver UI pages Summary: The on-disk size breakdown shown on the yb-tserver and yb-master web UI pages did not account for vector indexes, so a table with a vector index under-reported its actual disk footprint. Add VectorLSM::OnDiskSize, which sums the sizes of the immutable chunk files currently on disk, and expose it through DocVectorIndex::OnDiskSize and VectorIndexList::OnDiskSize. TabletPeer aggregates the per-tablet vector index size into a new TabletOnDiskSizeInfo::vector_index_disk_size field, which is folded into active_on_disk_size and serialized in TabletStatusPB. Propagate the size to the master via a new vector_index_size field in TabletDriveStorageMetadataPB and TabletReplicaDriveInfo. The tserver tables/tablets pages and the master tables page now render a "Vector Indexes" line in the size breakdown, and the corresponding JSON endpoints expose vector_index_size. The HTML line is rendered only when the size is non-zero, so tables without a vector index are not cluttered with "Vector Indexes: 0B". Also make TabletVectorIndexes::List, TabletVectorIndexes::Collect, and TabletComponent::VectorIndexesList return the VectorIndexList wrapper instead of the raw docdb::DocVectorIndexesPtr. This removes the repeated VectorIndexList(...) wrapping at call sites; the few places that need the underlying pointer for the docdb write/apply path use the new VectorIndexList::impl accessor. --- **Upgrade / Rollback safety:** Adds field used by new functionality. The old code would just ignore it. New code would not show usage if information is received from node with an old code. --- _automated · Claude Code (Opus 4.8)_ Original commit: 3b4a0bafbf0850cbc7ae982c0cebf89fcca6021a / D54922 Test Plan: ./yb_build.sh debug --cxx-test pg_vector_index-test --gtest_filter 'PgVectorIndexTest.OnDiskSize*' Manually verified on a local cluster: a table with an ybhnsw vector index reports a non-zero "Vector Indexes" size on both the tserver (:9000) and master (:7000) UI pages and in their JSON endpoints, while a table without a vector index reports none. Reviewers: arybochkin Reviewed By: arybochkin Subscribers: yql, ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D55013
| Commit: | 1e2414b | |
|---|---|---|
| Author: | Sumukh-Phalgaonkar | |
| Committer: | Sumukh-Phalgaonkar | |
[BACKPORT 2026.1][#32116] CDC: Add support to create table bound gRPC streams Summary: ### Backport Description No merge conflicts ### Original Description ##### Code changes summary Currently when a replication slot is created, retention barriers are setup and cdc_state table entries are written for all the tablets in the DB. In an environment where large number of tables are present and only a small subset is being used for CDC, this leads to unnecessary retention barrier setup. Also the cdc_state table is bloated with unnecessary entries. To prevent this, this diff introduces a mechanism to create gRPC streams that are bound to only specific tables at the time of their creation. To create such a stream, a comma separated list of table ids should be provided to the create_change_data_stream yb-admin command. The syntax is as follows: ``` ./yb-admin create_change_data_stream ysql.<DB-name> EXPLICIT CHANGE NOEXPORT_SNAPSHOT DYNAMIC_TABLES_DISABLED <comma separated table_ids> ``` For example: ``` ./yb-admin create_change_data_stream ysql.yugabyte EXPLICIT CHANGE NOEXPORT_SNAPSHOT DYNAMIC_TABLES_DISABLED 000034e1000030008000000000004000,000034e1000030008000000000004005 CDC Stream ID: 9fbec9b0395a2caacd48676d714ced0d ``` The table_ids are passed to the `CreateCDCStream` rpc by populating the `bound_table_ids` field in the `CDCSDKStreamCreateOptionsPB`. Only these table_ids are written to the stream metadata. The retention barriers are set on the tablets of only these tables, and their entries are written to the cdc_state table. Dynamic table addition is disabled for the table bound streams, meaning that the tables which can be polled using these streams is fixed at the stream creation. Any attmept to create such streams with dynamic tables enabled will fail. Also such streams can only be created for gRPC model. ##### Upgrade / Rollback safety Only proto change made in this diff is in `CDCSDKStreamCreateOptionsPB` which is a part of `CreateCDCStreamRequestPB`. The CreateCDCStream rpc flows from the tserver to the master. Since all the masters are upgraded before the tservers, this change is upgrade safe. Additionally the bound_table_ids field added in `CDCSDKStreamCreateOptionsPB` is an optional field. To make the repeated field optional it has been wrapped in a separate proto called `CDCSDKBoundTableIds`. A table bound stream created before rollback will continue to operate as intended after rollback, i.e post rollback user can use the table bound stream to get the change events from the tables present in the stream metadata. Hence this change is rollback safe. ##### Considerations for colocated tables If a stream is created such that it is bound to subset of colocated tables residing on the tablet, cdcsdk_producer will filter out the change records corresponding to other tables. ##### Considerations for connector NA Original commit: 6d1ef2a86b3e1b0e3055ba1614944245c88c479e / D54632 Test Plan: New tests added: - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestgRPCStreamBoundToSpecificTables' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestgRPCStreamBoundToSpecificColocatedTables' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestTableBoundStreamRejectsWithReplicationSlot' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestTableBoundStreamDisablesDynamicAddition' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestCreationOfgRPCStreamBoundToSpecificTablesViaYBAdmin' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestTableBoundStreamYbAdminRejectsTableFromDifferentNamespace' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestTableBoundStreamYbAdminRejectsTableFromDifferentNamespaceTestTableBoundStreamYbAdminRejectsDynamicTablesEnabled' Reviewers: #db-approvers, xCluster, hsunder, skumar, asrinivasan, devansh.singhal Reviewed By: devansh.singhal Subscribers: ybase, svc_phabricator Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D55014
| Commit: | b61cc9f | |
|---|---|---|
| Author: | Anton Rybochkin | |
| Committer: | Anton Rybochkin | |
[#32360] DocDB: Vector index: add a new gflag and table parameter for vector reverse mapping ownership Summary: The change adds master-side gflag `enable_table_owned_vector_reverse_mapping` (default false), `owns_vector_reverse_mapping` on Schema::TableProperties, backup/restore fixup for that property, and plumbing of the flag/property into CompactionSchemaInfo and vector index backfill. The new parameter is set only during table creation and cannot be changed later. Additionally, `skip_reverse_mapping_backfill` is removed; backfill now follows the indexed table's `owns_vector_reverse_mapping` instead. Insert/update changes for vector reverse mapping ownership and compaction behavior are deferred to a follow-up revisions. **Upgrade/Rollback safety:** Guarded by master runtime gflag `enable_table_owned_vector_reverse_mapping`. Test Plan: ./yb_build.sh --cxx-test pg_vector_index-test --gtest_filter=PgVectorIndexUtilTest.BackfillSkipsReverseMapping ./yb_build.sh --cxx-test pg_vector_index-test --gtest_filter=PgVectorIndexUtilTest.BackfillWritesReverseMapping ./yb_build.sh --cxx-test pg_vector_index-test --gtest_filter=PgVectorIndexUtilTest.NumTopVectorsToRemoveExceedsResultEntries ./yb_build.sh --cxx-test yb-backup-cross-feature-test --gtest_filter=YBBackupTest.TestYSQLTableOwnedVectorReverseMapping Reviewers: sergei, hsunder Reviewed By: sergei, hsunder Subscribers: yql, ybase Differential Revision: https://phorge.dev.yugabyte.com/D54966
| Commit: | 88658bb | |
|---|---|---|
| Author: | Timur Yusupov | |
| Committer: | Timur Yusupov | |
[BACKPORT 2024.2][#27056] docdb: Fixed tablet split vs RBS from the follower race Summary: There is a possibility of tablet split vs RBS from follower race: 1. Parent tablet leader peers A-C accept a SPLIT_OP (op_id: 1.4). Leader (A) and 1st follower (B) apply SPLIT_OP, 2nd follower (C) doesn't apply it yet. 2. Parent tablet leader (node A) accepts CHANGE_CONFIG_OP (op_id: 1.5) to add a fourth peer (D) but doesn't apply it yet. 3. Parent tablet 2nd follower (C) still hasn't yet applied SPLIT_OP (1.4). 4. RBS for parent tablet peer (D) starts from the follower (C) and tablet metadata (tablet_data_state == TABLET_DATA_READY) is downloaded. 5. Parent tablet peers A-C completed applying the SPLIT_OP (1.4), child tablets have Raft config with 3 peers. 6. Parent tablet peers A-C apply CHANGE_CONFIG_OP (1.5) and now have committed Raft config with 4 peers. 7. Parent tablet peer D does local bootstrap and replays SPLIT_OP (1.4) as part of bootstrap. Due to tablet_data_state is TABLET_DATA_READY but not TABLET_DATA_SPLIT_COMPLETED replay does SPLIT_OP apply and creates child tablet peer. After that, 4th child tablet peer (D) is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader. This change fixes this race by rejecting RBS from the follower that is in progress of applying SPLIT_OP and RBS attempt will be retried later. Original commit: 78af3a208a6bfc005eedc0ed6e410d22f4d24758 / D48853 **Upgrade/Rollback safety:** New error code will be printed by old nodes as just number in case of RBS failure during upgrade but this is safe. Test Plan: TabletSplitITest.SplitWithParentTabletRbsFromFollower, TabletSplitITest.SplitWithParentTabletMove, RemoteBootstrapsFromNodeWithUncommittedSplitOp - 30 runs per each of asan/tsan/debug/release builds Reviewers: arybochkin Reviewed By: arybochkin Subscribers: ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D54972
| Commit: | 37a83d7 | |
|---|---|---|
| Author: | Timur Yusupov | |
| Committer: | Timur Yusupov | |
[BACKPORT 2025.1][#27056] docdb: Fixed tablet split vs RBS from the follower race Summary: There is a possibility of tablet split vs RBS from follower race: 1. Parent tablet leader peers A-C accept a SPLIT_OP (op_id: 1.4). Leader (A) and 1st follower (B) apply SPLIT_OP, 2nd follower (C) doesn't apply it yet. 2. Parent tablet leader (node A) accepts CHANGE_CONFIG_OP (op_id: 1.5) to add a fourth peer (D) but doesn't apply it yet. 3. Parent tablet 2nd follower (C) still hasn't yet applied SPLIT_OP (1.4). 4. RBS for parent tablet peer (D) starts from the follower (C) and tablet metadata (tablet_data_state == TABLET_DATA_READY) is downloaded. 5. Parent tablet peers A-C completed applying the SPLIT_OP (1.4), child tablets have Raft config with 3 peers. 6. Parent tablet peers A-C apply CHANGE_CONFIG_OP (1.5) and now have committed Raft config with 4 peers. 7. Parent tablet peer D does local bootstrap and replays SPLIT_OP (1.4) as part of bootstrap. Due to tablet_data_state is TABLET_DATA_READY but not TABLET_DATA_SPLIT_COMPLETED replay does SPLIT_OP apply and creates child tablet peer. After that, 4th child tablet peer (D) is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader. This change fixes this race by rejecting RBS from the follower that is in progress of applying SPLIT_OP and RBS attempt will be retried later. Original commit: 78af3a208a6bfc005eedc0ed6e410d22f4d24758 / D48853 **Upgrade/Rollback safety:** New error code will be printed by old nodes as just number in case of RBS failure during upgrade but this is safe. Test Plan: TabletSplitITest.SplitWithParentTabletRbsFromFollower, TabletSplitITest.SplitWithParentTabletMove, RemoteBootstrapsFromNodeWithUncommittedSplitOp - 30 runs per each of asan/tsan/debug/release builds Reviewers: arybochkin Reviewed By: arybochkin Subscribers: ybase, zdrudi Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D54946
| Commit: | acfe878 | |
|---|---|---|
| Author: | Timur Yusupov | |
| Committer: | Timur Yusupov | |
[BACKPORT 2025.2][#27056] docdb: Fixed tablet split vs RBS from the follower race Summary: There is a possibility of tablet split vs RBS from follower race: 1. Parent tablet leader peers A-C accept a SPLIT_OP (op_id: 1.4). Leader (A) and 1st follower (B) apply SPLIT_OP, 2nd follower (C) doesn't apply it yet. 2. Parent tablet leader (node A) accepts CHANGE_CONFIG_OP (op_id: 1.5) to add a fourth peer (D) but doesn't apply it yet. 3. Parent tablet 2nd follower (C) still hasn't yet applied SPLIT_OP (1.4). 4. RBS for parent tablet peer (D) starts from the follower (C) and tablet metadata (tablet_data_state == TABLET_DATA_READY) is downloaded. 5. Parent tablet peers A-C completed applying the SPLIT_OP (1.4), child tablets have Raft config with 3 peers. 6. Parent tablet peers A-C apply CHANGE_CONFIG_OP (1.5) and now have committed Raft config with 4 peers. 7. Parent tablet peer D does local bootstrap and replays SPLIT_OP (1.4) as part of bootstrap. Due to tablet_data_state is TABLET_DATA_READY but not TABLET_DATA_SPLIT_COMPLETED replay does SPLIT_OP apply and creates child tablet peer. After that, 4th child tablet peer (D) is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader. This change fixes this race by rejecting RBS from the follower that is in progress of applying SPLIT_OP and RBS attempt will be retried later. Original commit: 78af3a208a6bfc005eedc0ed6e410d22f4d24758 / D48853 **Upgrade/Rollback safety:** New error code will be printed by old nodes as just number in case of RBS failure during upgrade but this is safe. Test Plan: TabletSplitITest.SplitWithParentTabletRbsFromFollower, TabletSplitITest.SplitWithParentTabletMove, RemoteBootstrapsFromNodeWithUncommittedSplitOp - 30 runs per each of asan/tsan/debug/release builds Reviewers: arybochkin Reviewed By: arybochkin Subscribers: zdrudi, ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D54937
| Commit: | e2a2c1a | |
|---|---|---|
| Author: | Gaurav Singh | |
| Committer: | Gaurav Singh | |
[BACKPORT 2026.1][#30642] YSQL: Adding column tablet_state, oid and making yb_tablet_metadata view global. Summary: `yb_tablet_metadata` view only shows database specific tablet info along with showcasing tablet info for tables with same `relname` in other databases. Intended behavior for `yb_tablet_metadata`: The view should showcase cluster-wide tablet metadata. All YCQL tablets and YSQL tablets from schema `pg_catalog` and `information_schema` to be excluded. Include `system.transactions` in the view. **FIX:** On the backend, a new `CatalogManager::GetTablets()` method iterates the master's `tablet_map_` directly instead of going through `GetTables()` → `table->GetTablets()`. This ensures colocated tables sharing a physical tablet produce a single row (the colocation parent) rather than duplicate rows per user table. Although, this fix will let any user see `relname` across the cluster without any restrictions. Masking will be enabled in the next diff. A new column `tablet_state` has been added and populated. This field can have possible values as `PREPARING`, `CREATING`, `REPLACED`, `RUNNING` and `DELETED`. NOTE: Tombstoned tablets (REPLACED or DELETED state) are still shown in the function/view up until they are removed by compaction. To avoid creating a new migration in the next diff where the function will have 3 new columns (`start_range`, `end_range`, `tablet_attrs`), made necessary changes in the migration file and marked them as nulls in `pg_yb_utils.c`. The subsequent revision will populate these columns. Also added a new field `oid` to the function. The `oid` in the view previously showed `relfilenode` with was derived from `object_uuid`. **Upgrade/Rollback safety:** This diff adds a new optional fields `tablet_state` and `pg_table_oid` in the protobuf message. This change is safe to upgrade/rollback. Original commit: ac7942d0357d9b4764eb05c21bf6cb7a7b9b78b9 / D52928 Test Plan: ./yb_build.sh release --java-test TestPgRegressMisc#testPgRegressMiscIndependent ./yb_build.sh release --java-test 'org.yb.pgsql.TestPgRegressRules#testPgRegressRules' ./yb_build.sh release --java-test TestYsqlUpgrade#migratingIsEquivalentToReinitdb ./yb_build.sh release --cxx-test yql-test --gtest_filter YqlTest.TabletMetadataViewsWithYcqlAndYsql ./yb_build.sh release --cxx-test pg_conn-test --gtest_filter PgConnTest.TabletMetadataConnectWithLeader ./yb_build.sh release --cxx-test pg_mini-test --gtest_filter PgMiniTest.TabletMetadataOidMatchesPgClass ./yb_build.sh release --cxx-test pg_mini-test --gtest_filter PgMiniTest.TabletMetadataCorrectnessWithHashPartitioning ./yb_build.sh release --cxx-test pg_mini-test --gtest_filter PgMiniTest.TabletMetadataStateColumn Reviewers: ishan.chhangani, asaha, cagrawal, aman.mangal, swapnil.kasaliwal Reviewed By: ishan.chhangani Subscribers: ybase, yql Differential Revision: https://phorge.dev.yugabyte.com/D55007
| Commit: | 1685c0e | |
|---|---|---|
| Author: | Gaurav Singh | |
| Committer: | Gaurav Singh | |
[BACKPORT 2025.2][#30642] YSQL: Adding column tablet_state, oid and making yb_tablet_metadata view global. Summary: `yb_tablet_metadata` view only shows database specific tablet info along with showcasing tablet info for tables with same `relname` in other databases. Intended behavior for `yb_tablet_metadata`: The view should showcase cluster-wide tablet metadata. All YCQL tablets and YSQL tablets from schema `pg_catalog` and `information_schema` to be excluded. Include `system.transactions` in the view. **FIX:** On the backend, a new `CatalogManager::GetTablets()` method iterates the master's `tablet_map_` directly instead of going through `GetTables()` → `table->GetTablets()`. This ensures colocated tables sharing a physical tablet produce a single row (the colocation parent) rather than duplicate rows per user table. Although, this fix will let any user see `relname` across the cluster without any restrictions. Masking will be enabled in the next diff. A new column `tablet_state` has been added and populated. This field can have possible values as `PREPARING`, `CREATING`, `REPLACED`, `RUNNING` and `DELETED`. NOTE: Tombstoned tablets (REPLACED or DELETED state) are still shown in the function/view up until they are removed by compaction. To avoid creating a new migration in the next diff where the function will have 3 new columns (`start_range`, `end_range`, `tablet_attrs`), made necessary changes in the migration file and marked them as nulls in `pg_yb_utils.c`. The subsequent revision will populate these columns. Also added a new field `oid` to the function. The `oid` in the view previously showed `relfilenode` which was derived from `object_uuid`. **Upgrade/Rollback safety:** This diff adds a new optional fields `tablet_state` and `pg_table_oid` in the protobuf message. This change is safe to upgrade/rollback. Original commit: ac7942d0357d9b4764eb05c21bf6cb7a7b9b78b9 / D52928 Test Plan: ./yb_build.sh release --java-test TestPgRegressMisc#testPgRegressMiscIndependent ./yb_build.sh release --java-test 'org.yb.pgsql.TestPgRegressRules#testPgRegressRules' ./yb_build.sh release --java-test TestYsqlUpgrade#migratingIsEquivalentToReinitdb ./yb_build.sh release --cxx-test yql-test --gtest_filter YqlTest.TabletMetadataViewsWithYcqlAndYsql ./yb_build.sh release --cxx-test pg_conn-test --gtest_filter PgConnTest.TabletMetadataConnectWithLeader ./yb_build.sh release --cxx-test pg_mini-test --gtest_filter PgMiniTest.TabletMetadataOidMatchesPgClass ./yb_build.sh release --cxx-test pg_mini-test --gtest_filter PgMiniTest.TabletMetadataCorrectnessWithHashPartitioning ./yb_build.sh release --cxx-test pg_mini-test --gtest_filter PgMiniTest.TabletMetadataStateColumn Reviewers: ishan.chhangani, asaha, cagrawal, swapnil.kasaliwal, aman.mangal Reviewed By: ishan.chhangani Subscribers: yql, ybase Differential Revision: https://phorge.dev.yugabyte.com/D55003
| Commit: | 3b4a0ba | |
|---|---|---|
| Author: | Sergei Politov | |
| Committer: | Sergei Politov | |
[#30883] DocDB: Show vector index space usage in yb-master and yb-tserver UI pages Summary: The on-disk size breakdown shown on the yb-tserver and yb-master web UI pages did not account for vector indexes, so a table with a vector index under-reported its actual disk footprint. Add VectorLSM::OnDiskSize, which sums the sizes of the immutable chunk files currently on disk, and expose it through DocVectorIndex::OnDiskSize and VectorIndexList::OnDiskSize. TabletPeer aggregates the per-tablet vector index size into a new TabletOnDiskSizeInfo::vector_index_disk_size field, which is folded into active_on_disk_size and serialized in TabletStatusPB. Propagate the size to the master via a new vector_index_size field in TabletDriveStorageMetadataPB and TabletReplicaDriveInfo. The tserver tables/tablets pages and the master tables page now render a "Vector Indexes" line in the size breakdown, and the corresponding JSON endpoints expose vector_index_size. The HTML line is rendered only when the size is non-zero, so tables without a vector index are not cluttered with "Vector Indexes: 0B". Also make TabletVectorIndexes::List, TabletVectorIndexes::Collect, and TabletComponent::VectorIndexesList return the VectorIndexList wrapper instead of the raw docdb::DocVectorIndexesPtr. This removes the repeated VectorIndexList(...) wrapping at call sites; the few places that need the underlying pointer for the docdb write/apply path use the new VectorIndexList::impl accessor. --- **Upgrade / Rollback safety:** Adds field used by new functionality. The old code would just ignore it. New code would not show usage if information is received from node with an old code. --- _automated · Claude Code (Opus 4.8)_ Test Plan: ./yb_build.sh debug --cxx-test pg_vector_index-test --gtest_filter 'PgVectorIndexTest.OnDiskSize*' Manually verified on a local cluster: a table with an ybhnsw vector index reports a non-zero "Vector Indexes" size on both the tserver (:9000) and master (:7000) UI pages and in their JSON endpoints, while a table without a vector index reports none. Reviewers: arybochkin Reviewed By: arybochkin Subscribers: ybase, yql Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D54922
| Commit: | ac7942d | |
|---|---|---|
| Author: | Gaurav Singh | |
| Committer: | Gaurav Singh | |
[#30642] YSQL: Adding column tablet_state, oid and making yb_tablet_metadata view global. Summary: ## SUMMARY `yb_tablet_metadata` view only shows database specific tablet info along with showcasing tablet info for tables with same `relname` in other databases. Intended behavior for `yb_tablet_metadata`: The view should showcase cluster-wide tablet metadata. All YCQL tablets and YSQL tablets from schema `pg_catalog` and `information_schema` to be excluded. Include `system.transactions` in the view. **FIX:** On the backend, a new `CatalogManager::GetTablets()` method iterates the master's `tablet_map_` directly instead of going through `GetTables()` → `table->GetTablets()`. This ensures colocated tables sharing a physical tablet produce a single row (the colocation parent) rather than duplicate rows per user table. Although, this fix will let any user see `relname` across the cluster without any restrictions. Masking will be enabled in the next diff. A new column `tablet_state` has been added and populated. This field can have possible values as `PREPARING`, `CREATING`, `REPLACED`, `RUNNING` and `DELETED`. NOTE: Tombstoned tablets (REPLACED or DELETED state) are still shown in the function/view up until they are removed by compaction. ### Additional changes To avoid creating a new migration in the next diff where the function will have 3 new columns (`start_range`, `end_range`, `tablet_attrs`), made necessary changes in the migration file and marked them as nulls in `pg_yb_utils.c`. The subsequent revision will populate these columns. Also added a new field `oid` to the function. The `oid` in the view previously showed `relfilenode` with was derived from `object_uuid`. **Upgrade/Rollback safety:** This diff adds a new optional fields `tablet_state` and `pg_table_oid` in the protobuf message. This change is safe to upgrade/rollback. Test Plan: ./yb_build.sh release --java-test TestPgRegressMisc#testPgRegressMiscIndependent ./yb_build.sh release --java-test 'org.yb.pgsql.TestPgRegressRules#testPgRegressRules' ./yb_build.sh release --java-test TestYsqlUpgrade#migratingIsEquivalentToReinitdb ./yb_build.sh release --cxx-test yql-test --gtest_filter YqlTest.TabletMetadataViewsWithYcqlAndYsql ./yb_build.sh release --cxx-test pg_conn-test --gtest_filter PgConnTest.TabletMetadataConnectWithLeader ./yb_build.sh release --cxx-test pg_mini-test --gtest_filter PgMiniTest.TabletMetadataOidMatchesPgClass ./yb_build.sh release --cxx-test pg_mini-test --gtest_filter PgMiniTest.TabletMetadataCorrectnessWithHashPartitioning ./yb_build.sh release --cxx-test pg_mini-test --gtest_filter PgMiniTest.TabletMetadataStateColumn Reviewers: ishan.chhangani, kfranz Reviewed By: kfranz Subscribers: ybase, yql Differential Revision: https://phorge.dev.yugabyte.com/D52928
| Commit: | 38d8291 | |
|---|---|---|
| Author: | Timur Yusupov | |
| Committer: | Timur Yusupov | |
[BACKPORT 2026.1][#27056] docdb: Fixed tablet split vs RBS from the follower race Summary: There is a possibility of tablet split vs RBS from follower race: 1. Parent tablet leader peers A-C accept a SPLIT_OP (op_id: 1.4). Leader (A) and 1st follower (B) apply SPLIT_OP, 2nd follower (C) doesn't apply it yet. 2. Parent tablet leader (node A) accepts CHANGE_CONFIG_OP (op_id: 1.5) to add a fourth peer (D) but doesn't apply it yet. 3. Parent tablet 2nd follower (C) still hasn't yet applied SPLIT_OP (1.4). 4. RBS for parent tablet peer (D) starts from the follower (C) and tablet metadata (tablet_data_state == TABLET_DATA_READY) is downloaded. 5. Parent tablet peers A-C completed applying the SPLIT_OP (1.4), child tablets have Raft config with 3 peers. 6. Parent tablet peers A-C apply CHANGE_CONFIG_OP (1.5) and now have committed Raft config with 4 peers. 7. Parent tablet peer D does local bootstrap and replays SPLIT_OP (1.4) as part of bootstrap. Due to tablet_data_state is TABLET_DATA_READY but not TABLET_DATA_SPLIT_COMPLETED replay does SPLIT_OP apply and creates child tablet peer. After that, 4th child tablet peer (D) is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader. This change fixes this race by rejecting RBS from the follower that is in progress of applying SPLIT_OP and RBS attempt will be retried later. Original commit: 78af3a208a6bfc005eedc0ed6e410d22f4d24758 / D48853 **Upgrade/Rollback safety:** New error code will be printed by old nodes as just number in case of RBS failure during upgrade but this is safe. Test Plan: TabletSplitITest.SplitWithParentTabletRbsFromFollower, TabletSplitITest.SplitWithParentTabletMove, RemoteBootstrapsFromNodeWithUncommittedSplitOp - 30 runs per each of asan/tsan/debug/release builds Reviewers: arybochkin Reviewed By: arybochkin Subscribers: ybase, zdrudi Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D54936
| Commit: | ba7d8ab | |
|---|---|---|
| Author: | Fizaa Luthra | |
| Committer: | Fizaa Luthra | |
[pg19] cdc: stop reading removed pg_attribute.attcacheoff Summary: Upstream PG commit 02a8d0c45253eb54e57b1974c8627e5be3e1d852 ("Remove pg_attribute.attcacheoff column") removed the attcacheoff column. SysCatalogTable::ReadPgAttributeInfo still looked it up via ColumnIdByName("attcacheoff") -> "Couldn't find column attcacheoff in the schema" (common/schema.cc), which fails the GetUDTypeMetadata RPC. PgAttributePB.attcacheoff is a required proto field consumed by CDC. Dropping or relaxing the field would break cross-version CDC/xCluster: a pre-PG19 peer still compiled with attcacheoff as required fails to parse any message that omits it. Keep the field as required, stop reading the now-missing column in ReadPgAttributeInfo and set attcacheoff = -1. Upgrade/Rollback safety: The .proto change is comment-only -- no field is added, removed, renumbered, or retyped -- so the wire format is unchanged: PgAttributePB.attcacheoff stays a required int32 (tag 8). The only functional change (sys_catalog.cc) emits a constant 0 for that field instead of reading the now-removed catalog column. Test Plan: ./yb_build.sh release --cxx-test cdcsdk_consumption_consistent_changes-test --gtest_filter 'CDCSDKConsumptionConsistentChangesTest.TestCompactionWithReplicaIdentityDefault Reviewers: aagrawal Reviewed By: aagrawal Subscribers: sumukh.phalgaonkar, ybase Differential Revision: https://phorge.dev.yugabyte.com/D54855
| Commit: | b0e0f0a | |
|---|---|---|
| Author: | Bvsk Patnaik | |
| Committer: | Bvsk Patnaik | |
[BACKPORT 2026.1][#31166] YSQL: Fix follower read time for parallel queries Summary: Original commit: 90f9f34b21ccfa980c8ffc4c814c0f9a0e7d3b3b / D52581 #### Problem Below is the log summary of a parallel query with vmodule=pg_session=2,pg_client_session=2 ``` # Leader (PID 589293) launches 2 parallel workers 04:36:25.391 [589293] DEBUG: YB: launching parallel workers 04:36:25.392 [589293] DEBUG: YB: launched 2 parallel workers # pg_session.cc: Leader sets read_time from follower read staleness 04:36:25.391 589293 pg_session.cc:900 Perform options: read_ht: 7281091302930841600 (physical: 1777610181379600) read_from_followers: true, read_time_serial_no: 50 # pg_session.cc: Worker 1 independently computes its own read_time (+46ms) 04:36:25.459 589717 pg_session.cc:900 Perform options: read_ht: 7281091303120982016 (physical: 1777610181426021) read_from_followers: true, read_time_serial_no: 50 # pg_session.cc: Worker 2 independently computes its own read_time (+56ms) 04:36:25.460 589718 pg_session.cc:900 Perform options: read_ht: 7281091303160123392 (physical: 1777610181435577) read_from_followers: true, read_time_serial_no: 50 # pg_client_session.cc: tserver receives all three distinct read_times # through the SAME session (pid 589293), confirming it uses whatever # read_time each worker specified: 04:36:25.393 Session id 6 (pid 589293): read_time={ physical: 1777610181379600 } # leader 04:36:25.460 Session id 6 (pid 589293): read_time={ physical: 1777610181426021 } # worker 1 04:36:25.460 Session id 6 (pid 589293): read_time={ physical: 1777610181435577 } # worker 2 # All workers finish 04:36:25.470 [589293] DEBUG: YB: all 2 parallel workers finished ``` From the log, we can see that all the workers use different read time. This can potentially cause an inconsistent read. #### Root Cause Follower reads computed their read time (now - staleness) in the YSQL backend using the backend's own clock. Each parallel worker is a separate backend, so workers in the same query computed different read times and could return inconsistent results. #### Fix Pick the read time in PgClientSession. When the PgClientSession picks the read point it lowers it by that staleness (SetFollowerReadTime). Every perform and parallel worker that shares the serial number - reads at the same time. Moreover, clock usage is removed from PgTxnManager. This is a step forward in removal of HybridClock from postgres backend since postgres backend is not involved in hybrid time propagation. PgApiImpl keeps a HybridClock only for ANALYZE sampling; see #16034. **Upgrade/Rollback safety:** Only changes proto used for communication between postgres and local tserver proxy. Test Plan: Jenkins ./yb_build.sh release --java-test 'org.yb.pgsql.TestPgFollowerReads#testBankInvariantWithParallelFollowerReads' Fails with the error below without the fix ``` INCONSISTENCY: expected total=100000 but got 100004 (diff=+4) after 611 transfers INCONSISTENCY: expected total=100000 but got 99993 (diff=-7) after 742 transfers INCONSISTENCY: expected total=100000 but got 99988 (diff=-12) after 1065 transfers INCONSISTENCY: expected total=100000 but got 99999 (diff=-1) after 1213 transfers INCONSISTENCY: expected total=100000 but got 100007 (diff=+7) after 1372 transfers INCONSISTENCY: expected total=100000 but got 100007 (diff=+7) after 1645 transfers ``` Reviewers: pjain, amartsinchyk, sanketh, smishra, dmitry Reviewed By: pjain Subscribers: yql, ybase, mtakahara Differential Revision: https://phorge.dev.yugabyte.com/D54934
| Commit: | 90f9f34 | |
|---|---|---|
| Author: | Bvsk Patnaik | |
| Committer: | Bvsk Patnaik | |
[#31166] YSQL: Fix follower read time for parallel queries Summary: #### Problem Below is the log summary of a parallel query with vmodule=pg_session=2,pg_client_session=2 ``` # Leader (PID 589293) launches 2 parallel workers 04:36:25.391 [589293] DEBUG: YB: launching parallel workers 04:36:25.392 [589293] DEBUG: YB: launched 2 parallel workers # pg_session.cc: Leader sets read_time from follower read staleness 04:36:25.391 589293 pg_session.cc:900 Perform options: read_ht: 7281091302930841600 (physical: 1777610181379600) read_from_followers: true, read_time_serial_no: 50 # pg_session.cc: Worker 1 independently computes its own read_time (+46ms) 04:36:25.459 589717 pg_session.cc:900 Perform options: read_ht: 7281091303120982016 (physical: 1777610181426021) read_from_followers: true, read_time_serial_no: 50 # pg_session.cc: Worker 2 independently computes its own read_time (+56ms) 04:36:25.460 589718 pg_session.cc:900 Perform options: read_ht: 7281091303160123392 (physical: 1777610181435577) read_from_followers: true, read_time_serial_no: 50 # pg_client_session.cc: tserver receives all three distinct read_times # through the SAME session (pid 589293), confirming it uses whatever # read_time each worker specified: 04:36:25.393 Session id 6 (pid 589293): read_time={ physical: 1777610181379600 } # leader 04:36:25.460 Session id 6 (pid 589293): read_time={ physical: 1777610181426021 } # worker 1 04:36:25.460 Session id 6 (pid 589293): read_time={ physical: 1777610181435577 } # worker 2 # All workers finish 04:36:25.470 [589293] DEBUG: YB: all 2 parallel workers finished ``` From the log, we can see that all the workers use different read time. This can potentially cause an inconsistent read. #### Root Cause Follower reads computed their read time (now - staleness) in the YSQL backend using the backend's own clock. Each parallel worker is a separate backend, so workers in the same query computed different read times and could return inconsistent results. #### Fix Pick the read time in PgClientSession. When the PgClientSession picks the read point it lowers it by that staleness (SetFollowerReadTime). Every perform and parallel worker that shares the serial number - reads at the same time. Moreover, clock usage is removed from PgTxnManager. This is a step forward in removal of HybridClock from postgres backend since postgres backend is not involved in hybrid time propagation. PgApiImpl keeps a HybridClock only for ANALYZE sampling; see #16034. **Upgrade/Rollback safety:** Only changes proto used for communication between postgres and local tserver proxy. Test Plan: Jenkins ./yb_build.sh release --java-test 'org.yb.pgsql.TestPgFollowerReads#testBankInvariantWithParallelFollowerReads' Fails with the error below without the fix ``` INCONSISTENCY: expected total=100000 but got 100004 (diff=+4) after 611 transfers INCONSISTENCY: expected total=100000 but got 99993 (diff=-7) after 742 transfers INCONSISTENCY: expected total=100000 but got 99988 (diff=-12) after 1065 transfers INCONSISTENCY: expected total=100000 but got 99999 (diff=-1) after 1213 transfers INCONSISTENCY: expected total=100000 but got 100007 (diff=+7) after 1372 transfers INCONSISTENCY: expected total=100000 but got 100007 (diff=+7) after 1645 transfers ``` Reviewers: pjain, amartsinchyk, sanketh, smishra, dmitry Reviewed By: pjain, dmitry Subscribers: mtakahara, ybase, yql Differential Revision: https://phorge.dev.yugabyte.com/D52581
| Commit: | d5ce407 | |
|---|---|---|
| Author: | Samson Shaji | |
| Committer: | Samson Shaji | |
[#32004] DocDB: Return useful tablet metadata as a response to DeleteTablet Summary: - Extend `DeleteTablet` RPC response with tablet metadata to include the following so callers can verify after delete. Added previous/final data state, tablet_id, table name, hide_only, and per-directory paths with still_present_after. - Populate the response in `TSTabletManager::DeleteTablet` when an optional `DeleteTabletResponsePB*` is provided; existing internal callers unchanged (resp defaults to `nullptr`). - Update yb-ts-cli `delete_tablet` to print structured JSON on successful RPC (message, tablet_id, table_name, and details). **Upgrade/Rollback safety:** This change only adds optional fields to `DeleteTabletResponsePB` and `DeletedDirectoryPB` on the existing `DeleteTablet` admin RPC. It does not change request semantics, on-disk tablet metadata, catalog schema, or any format outside that response. Describe how this change handles upgrade and rollback of YugabyteDB. Only have optional fields on `DeleteTablet` response. Delete behaviour is unchanged. What Test/Preview/AutoFlag is used to guard the feature? No Test/Preview/AutoFlag, behavior is backward compatible by protobuf optional-field rules. Test Plan: Built using: ``` ./yb_build.sh debug ``` Tested output using: ``` $PWD/build/latest/bin/yb-ts-cli --server_address=127.0.0.1:9100 delete_tablet '<tablet_id' 'testing on feature branch' ``` Screenshots of change. Before: {F501795} After: {F501796} Also ran the following test suite: ``` ./yb_build.sh debug --cxx-test delete_table-test ``` Reviewers: mhaddad, bkolagani Reviewed By: mhaddad Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D54131
| Commit: | 6d1ef2a | |
|---|---|---|
| Author: | Sumukh-Phalgaonkar | |
| Committer: | Sumukh-Phalgaonkar | |
[#32116] CDC: Add support to create table bound gRPC streams Summary: ##### Code changes summary Currently when a replication slot is created, retention barriers are setup and cdc_state table entries are written for all the tablets in the DB. In an environment where large number of tables are present and only a small subset is being used for CDC, this leads to unnecessary retention barrier setup. Also the cdc_state table is bloated with unnecessary entries. To prevent this, this diff introduces a mechanism to create gRPC streams that are bound to only specific tables at the time of their creation. To create such a stream, a comma separated list of table ids should be provided to the create_change_data_stream yb-admin command. The syntax is as follows: ``` ./yb-admin create_change_data_stream ysql.<DB-name> EXPLICIT CHANGE NOEXPORT_SNAPSHOT DYNAMIC_TABLES_DISABLED <comma separated table_ids> ``` For example: ``` ./yb-admin create_change_data_stream ysql.yugabyte EXPLICIT CHANGE NOEXPORT_SNAPSHOT DYNAMIC_TABLES_DISABLED 000034e1000030008000000000004000,000034e1000030008000000000004005 CDC Stream ID: 9fbec9b0395a2caacd48676d714ced0d ``` The table_ids are passed to the `CreateCDCStream` rpc by populating the `bound_table_ids` field in the `CDCSDKStreamCreateOptionsPB`. Only these table_ids are written to the stream metadata. The retention barriers are set on the tablets of only these tables, and their entries are written to the cdc_state table. Dynamic table addition is disabled for the table bound streams, meaning that the tables which can be polled using these streams is fixed at the stream creation. Any attmept to create such streams with dynamic tables enabled will fail. Also such streams can only be created for gRPC model. ##### Upgrade / Rollback safety Only proto change made in this diff is in `CDCSDKStreamCreateOptionsPB` which is a part of `CreateCDCStreamRequestPB`. The CreateCDCStream rpc flows from the tserver to the master. Since all the masters are upgraded before the tservers, this change is upgrade safe. Additionally the bound_table_ids field added in `CDCSDKStreamCreateOptionsPB` is an optional field. To make the repeated field optional it has been wrapped in a separate proto called `CDCSDKBoundTableIds`. A table bound stream created before rollback will continue to operate as intended after rollback, i.e post rollback user can use the table bound stream to get the change events from the tables present in the stream metadata. Hence this change is rollback safe. ##### Considerations for colocated tables If a stream is created such that it is bound to subset of colocated tables residing on the tablet, cdcsdk_producer will filter out the change records corresponding to other tables. ##### Considerations for connector NA Test Plan: New tests added: - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestgRPCStreamBoundToSpecificTables' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestgRPCStreamBoundToSpecificColocatedTables' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestTableBoundStreamRejectsWithReplicationSlot' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestTableBoundStreamDisablesDynamicAddition' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestCreationOfgRPCStreamBoundToSpecificTablesViaYBAdmin' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestTableBoundStreamYbAdminRejectsTableFromDifferentNamespace' - ./yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter 'CDCSDKYsqlTest.TestTableBoundStreamYbAdminRejectsTableFromDifferentNamespaceTestTableBoundStreamYbAdminRejectsDynamicTablesEnabled' Reviewers: xCluster, hsunder, skumar, asrinivasan, devansh.singhal, #db-approvers Reviewed By: asrinivasan, #db-approvers Subscribers: svc_phabricator, ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D54632
| Commit: | 6420537 | |
|---|---|---|
| Author: | Anton Rybochkin | |
| Committer: | Anton Rybochkin | |
[BACKPORT 2025.2][#31958] docdb: Vector Index: Skip reverse mapping insertion during backfill Summary: Currently, vector index reverse mapping entries are inserted for all existing vectors whenever a new vector index is created. This approach inserts the same data repeatedly each time a new vector index is created for the same vector column, which is incorrect. Instead, the indexed table should own the reverse mapping and manage its insertion and deletion. Therefore, reverse mapping population should not happen during vector index backfill. This change addresses the backfill aspect and implements the logic to skip reverse mapping population while a vector index is being backfilled. The logic is currently disabled until the remaining parts of the reverse mapping ownership logic are implemented (https://github.com/yugabyte/yugabyte-db/issues/31886). For backward compatibility, `PgVectorIdxOptionsPB` is extended with a new field, `skip_reverse_mapping_backfill`, which is automatically unset in older releases because the field is not present there. This logic is required to ensure that the old approach is used when vector index backfill started before the upgrade (so some reverse mapping entries may have already been added) but had not yet completed by the time of the upgrade. Original commit: 390ba45656e51cb36ebaf821ceb87fe1a00e9d59 / D53921 **Upgrade/Rollback safety:** The change is backward compatible, and the absence of the new field is treated as the old behavior -- this is actually required to allow an in-progress backfill started before the upgrade to complete using the old approach, ensuring that no vectors are lost. Test Plan: yb_build.sh --cxx-test='TEST_F(PgVectorIndexUtilTest, BackfillSkipsReverseMapping)' yb_build.sh --cxx-test='TEST_F(PgVectorIndexUtilTest, BackfillWritesReverseMapping)' Reviewers: sergei, zdrudi Reviewed By: zdrudi Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D54693
| Commit: | a0ba9f1 | |
|---|---|---|
| Author: | Anton Rybochkin | |
| Committer: | Anton Rybochkin | |
[BACKPORT 2026.1][#31958] docdb: Vector Index: Skip reverse mapping insertion during backfill Summary: Currently, vector index reverse mapping entries are inserted for all existing vectors whenever a new vector index is created. This approach inserts the same data repeatedly each time a new vector index is created for the same vector column, which is incorrect. Instead, the indexed table should own the reverse mapping and manage its insertion and deletion. Therefore, reverse mapping population should not happen during vector index backfill. This change addresses the backfill aspect and implements the logic to skip reverse mapping population while a vector index is being backfilled. The logic is currently disabled until the remaining parts of the reverse mapping ownership logic are implemented (https://github.com/yugabyte/yugabyte-db/issues/31886). For backward compatibility, `PgVectorIdxOptionsPB` is extended with a new field, `skip_reverse_mapping_backfill`, which is automatically unset in older releases because the field is not present there. This logic is required to ensure that the old approach is used when vector index backfill started before the upgrade (so some reverse mapping entries may have already been added) but had not yet completed by the time of the upgrade. Original commit: 390ba45656e51cb36ebaf821ceb87fe1a00e9d59 / D53921 **Upgrade/Rollback safety:** The change is backward compatible, and the absence of the new field is treated as the old behavior -- this is actually required to allow an in-progress backfill started before the upgrade to complete using the old approach, ensuring that no vectors are lost. Test Plan: yb_build.sh --cxx-test='TEST_F(PgVectorIndexUtilTest, BackfillSkipsReverseMapping)' yb_build.sh --cxx-test='TEST_F(PgVectorIndexUtilTest, BackfillWritesReverseMapping)' Reviewers: sergei, zdrudi Reviewed By: zdrudi Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D54692
| Commit: | efa6e10 | |
|---|---|---|
| Author: | Naorem Khogendro Singh | |
| Committer: | Naorem Khogendro Singh | |
[BACKPORT 2026.1][PLAT-20583][PLAT-19353] Remove Ansible libraries from Yugabyte Anywhere image Summary: Original diffs: 1. https://phorge.dev.yugabyte.com/D52420 (305866f4e87833d4925f6f9c25c3589204b2c3bc) - Remove ansible. 2. https://phorge.dev.yugabyte.com/D52297 (7fc952448ae5e58ee1dab87e75bd5e1b6e013bfd) - Make node agent mandatory. 3. https://phorge.dev.yugabyte.com/D52422 (118e77ca6f22beecf4a83985ca45071c22583756) - Fixes for the changes. 4. https://phorge.dev.yugabyte.com/D52433 (840389244027c372e26ea81f80e3454f38ad6b1d) - UT fix followup. 5. https://phorge.dev.yugabyte.com/D53143 (36519c9313f862f6882e241d45a0a40cdb1dc68f) - ssk key rotation fix. 6. https://phorge.dev.yugabyte.com/D54133 (f67bca88bbb4aa9aaa42f73a10a3260ee52d069a) - Empty optional must be returned if node-agent is not active 7. https://phorge.dev.yugabyte.com/D54120 (d1a6791e0831c8063dfd00103e3c643323436255) - Adjust keep-alive time parameters to avoid GOAWAY received This is a prerequisite before removing ansible because if node-agent is not found, the code flow for configure defaults to ansible. If it is ignored, it can lead of skipping tasks that is very dangerous! Note: Node agent is already mandatory now. [PLAT-20583] Fix for leaked instances: Make node agent mandatory at all applicable call sites to avoid defaulting to the legacy code with ansible Local change was not in the commit. Instance must be terminated for non-onprem. UT fix caused by PLAT-20583 making node agent mandatory. Calls to node manager is skipped for gflags upgrade. This leak fix https://phorge.dev.yugabyte.com/D52422 adds back some calls as destroy is down to NodeManager to terminate VMs. [PLAT-19353] Remove Ansible libraries from Yugabyte Anywhere image Deleted all the references to ansible including roles, python files. A dummy install_ansible_requirements.sh is kept as it is invoked during build (workaround for now). Conflict resolution. Test Plan: Manually tested. Will also wait for itests. Manually tested. Passed UTs locally. itests must pass. Manual tests passed locally - edit, create, resize, vm image upgrade. Reviewers: amalyshev, spothuraju, skhilar, yshchetinin, nbhatia, vkumar, muthu, anijhawan, anabaria Reviewed By: amalyshev, anijhawan Subscribers: nikhil, yugaware Differential Revision: https://phorge.dev.yugabyte.com/D52824
| Commit: | 62358fa | |
|---|---|---|
| Author: | Craig Soules | |
| Committer: | Sanketh I | |
[BACKPORT 2026.1][#30578] YSQL: Reset auto-analyze mutation counts after a manual ANALYZE Summary: Reset auto-analyze mutation counters after user-initiated ANALYZE Original commit: b6bbc7a663df2252865be498931bd4484ac7c224 / #31849 No conflicts on backport ## Problem User-initiated ANALYZE does not reset the auto-analyze service's accumulated mutation count. The next periodic tick can therefore trigger an auto-analyze on a table the user just analyzed, wasting work. ## Solution Add ResetAutoAnalyzeMutationCounters RPC (pg_client.proto / pg_client_service) called from do_analyze_rel after a full ANALYZE. The RPC resets the YCQL service table mutations to 0 for the analyzed table. Reset is gated to match PostgreSQL's semantics for updating its changes_since_analyze counters (fires only when no column list given) and is suppressed for auto-analyze service's internal connections and other internal backends. Auto analyze continues to reset mutations using the existing logic to subtract mutations on its side. Mutations are stored in a separate YCQL table so the mutations update is not transactional. A failed manual ANALYZE can still reset counters to 0. Fixing this is tracked in #32081. ## Race handling Extract mutation-update logic into helper functions in pg_auto_analyze_table that use conditional YCQL writes for better reuse. - ResetPgAutoAnalyzeMutationCounts: sets mutations to 0 (IF EXISTS) - SubtractPgAutoAnalyzeMutationCounts: subtracts snapshot mutations with clamping. Emits two conditional writes per table — one sets to 0 if current < snapshot, another subtracts if current >= snapshot. This prevents the count from going negative when a manual-ANALYZE reset races with auto-analyze post-ANALYZE mutation subtraction. UpdateTableMutationsAfterAnalyze refactored to use SubtractPgAutoAnalyzeMutationCounts instead of building operations directly. ## Wire-format changes New RPC: PgClientService.ResetAutoAnalyzeMutationCounters (pg_client.proto). Additive only — no existing field numbers or messages altered. Upgrade/rollback safety: New RPC is between PG and local tserver so no upgrade/rollback issues. Test Plan: - PgAutoAnalyzeTest.ManualAnalyzeResetsMutationCount: covers ANALYZE, ANALYZE(col), VACUUM ANALYZE, VACUUM ANALYZE(col), asserting reset only when no column list provided. - PgAutoAnalyzeTest.InternalAnalyzeDoesNotResetMutationCount: confirms internal connections do not trigger reset. - PgAutoAnalyzeTest.ManualAnalyzePartitionedTableResetsPartitionMutationCounts: exercises the partitioned-table path. - Existing PgAutoAnalyzeTest cases continue to pass. Reviewers: kfranz, pjain Reviewed By: kfranz Differential Revision: https://phorge.dev.yugabyte.com/D54272
| Commit: | f6e5a0b | |
|---|---|---|
| Author: | Aleksandr Malyshev | |
| Committer: | Aleksandr Malyshev | |
[PLAT-21159] Allow audit log retention for certain amount of days Summary: Allow keeping audit logs on the DB nodes for compliancy This is a requirement from customers, who don't want to export audit logs to external systems, but want to still be compliant with security requirements. Basically, what it does is - before gzipping postgres (YSQL) or tserser (YCQl) logs we're copying over all the audit line logs from the file to ./audit/[ysql or ycql]/......audit.log file. Now, we zip this new audit log file as well as the original log file. Original log file will be cleaned up, while audit log gzipped file will be kept on the node until the configured amount of days pass - and deleted after that. Number of days are controlled via additional autid logs setting. If the setting is not configured (or is configured to 0) - we keep the old behaviour. Test Plan: Installed older YBA release Created universe. Configured YSQL audit logs via the UI. Upgraded YBA. Re-configured YSQL audit logs via the UI to set the retention inetrval to 1 day. Make sure log rotation script was updated + otel-collector/log_cleanup_env file contains the new setting. Wait for postgres log file to be gzipped. Make sure audit log file gzip was created as well. Wait for 1 day to pass. Make sure the audit log file was deleted after the script run. Reviewers: vbansal, #yba-api-review! Reviewed By: vbansal Subscribers: yugaware Differential Revision: https://phorge.dev.yugabyte.com/D54457
| Commit: | cea57c0 | |
|---|---|---|
| Author: | Hideaki Kimura | |
| Committer: | Hideaki Kimura | |
[BACKPORT 2025.1][#31951] yb-admin: Add a key range to get_table_hash Summary: get_table_hash hashes a whole table. To narrow a detected inconsistency (e.g. during xCluster verification) down to where the data diverged, allow the scan to be restricted to a logical partition-key sub-range. Add optional start_key / end_key arguments (raw partition keys, hex-encoded on the command line -- the same encoding shown as partition_key_start / partition_key_end by list_tablets). start_key is inclusive, end_key is exclusive; an empty bound means unbounded on that side. The range is logical, so it is cluster-independent: each cluster resolves it to whatever tablets it owns, which is what makes it usable for cross-cluster comparison even when tablet boundaries differ. - DumpTabletDataRequestPB gains start_key and end_key. - tablet::DumpTabletData builds each table's encoded bound as [table prefix][encoded partition key]: the table prefix (cotable_id / colocation_id bytes; empty for a non-colocated table) places the bound in this table's slice of the tablet, and the encoded partition key narrows within it. An empty user bound leaves that side at the iterator's natural table boundary. - yb-admin's client skips tablets that do not overlap the requested range and forwards the bounds unchanged to every overlapping tablet. A key range scopes a single table, so it requires a concrete table_id: combined with a colocation parent id (which hashes every table in the tablet) it would be ambiguous, and is rejected with InvalidArgument. Bad input is rejected up front rather than silently hashing the wrong range: the CLI rejects malformed hex and an inverted range (start_key >= end_key), and the server rejects a bound that is not a 2-byte hash for a hash-partitioned table. Builds on #31952 (D53893, landed), whose per-table scoping this composes with: pass a child colocated table id to hash one colocated table over a key range. For #31951. **Upgrade/Rollback safety:** No persistent or on-disk format change, and no AutoFlag is needed. start_key and end_key are optional fields on DumpTabletDataRequestPB, an on-demand admin RPC used only by `yb-admin get_table_hash`; they are not written to disk, the WAL, or sys.catalog, and are absent from any consensus/replication path. Mixed-version behavior is safe: an old yb-admin sends no bounds, so a new tserver scans the full table (unchanged); a new yb-admin's bounds are ignored by an old tserver, which scans the full table rather than erroring -- run a version-matched yb-admin when using start_key/end_key. No state is persisted, so rollback has nothing to undo. Original commit: 208a0f88102d23f40365f4ba3d2c5005eddee88e / D53900 Test Plan: AdminCliTest.TestGetTableXorHashKeyRange (non-colocated YCQL hash table): explicit empty bounds reproduce the full-table totals, and a complementary 0x8000 split partitions the rows so counts sum and hashes XOR back to the full totals. PgLibPqTest.TestGetTableXorHashColocatedKeyRange (colocated, range-only table): derives real mid-data split keys for id=4 and id=8 from the server's own partitioning (a throwaway non-colocated SPLIT AT VALUES ((4), (8)) table), then -- passing the child colocated table id -- splits the table into three disjoint segments [-inf, key(4)) / [key(4), key(8)) / [key(8), +inf) (the middle one specifies both bounds), verifying exact per-segment row counts (3/4/3) and that the segments recombine (counts sum, hashes XOR) to the full totals. Also asserts that a key range against the colocation parent table id is rejected. Verified locally (debug/clang21): AdminCliTest.TestGetTableXorHashKeyRange passes. Relying on CSI for the full suite (incl. PgLibPqTest.TestGetTableXorHashColocatedKeyRange). Reviewers: jhe, #db-approvers Reviewed By: jhe, #db-approvers Subscribers: svc_phabricator Differential Revision: https://phorge.dev.yugabyte.com/D54292
| Commit: | 4043557 | |
|---|---|---|
| Author: | Basava | |
| Committer: | Basava Kolagani | |
[#27119] DocDB: Table locks: Introduce support for WaitForLockers Summary: **Background** Postgres supports a functionality `WaitForLockers` where the backend actively waits for other backends with active conflicting locks on the desired objects, and returns once the earlier snapshotted backends don't hold conflicting locks anymore. Currently, it is being invoked on the following paths 1. `REINDEX CONCURRENTLY` - YB doesn't support this as of today 2. `DROP INDEX CONCURRENTLY` - again, YB doesn't support this as of today. 3. `CREATE INDEX CONCURRENTLY` - we seem to be executing `WaitForLockers` only when `!IsYugaByteEnabled()` is true. Ideally, we would want to replace the current WaitForYsqlBackends on the concurrent index creation path with WaitForLockers some day (the former is a more generic version of the latter - WaitForYsqlBackends waits for ALL backends with stale catalog version, while WaitForLockers would wait for only backends with current active conflicting locks on desired objects). Created https://github.com/yugabyte/yugabyte-db/issues/31534 to track this effort. 4. `ALTER TABLE DETACH PARTITION CONCURRENTLY` - this seems to be the only active usage of the api. Put up a test case which fails without the functionality - the DML could have a stale view of the table metadata etc, but couldn't produce an data inconsistency issue without the support for WaitForLockers **Solution** **//WaitForLockers//** This revision introduces support for `WaitForLockers` api. For global lock acquires, a working mechanism exists where the host tserver forwards the lock request to the master, and the master then fans it out to all tservers with live ysql lease, and then keeps retrying until the ysql lease is valid or an ack is received. The same is leveraged for `WaitForLockers` as well as follows, 1. Master leader fans out the `WaitForLockers` request to every tserver with a valid ysql lease 2. The tserver then takes a snapshot of the active transactions with conflicting locks on the desired objects, and registers a shared callback that gets invoked on the lock release path for these transactions (which happens after transactions finish). 3. The tserver acks to the master after all such transactions finish. We could have some false positive transactions recorded in step 2. If the backend is on a later catalog version than that calling `WaitForLockers`, then the txn corresponding to the backend is not relevant for us. Yet, we wait on that txn even when we need not do so (since there's no real easy way to distinguish this). Created https://github.com/yugabyte/yugabyte-db/issues/31803 to track this effort. //Failure semantics// - If the tserver loses its ysql lease, then the master stops retrying the req at this tserver since existing backends would be killed and new ones would get the necessary invalidation messages on later lock acquires. - If the master loses it leadership by the time all `WaitForLockers` return from the tservers, it returns an error to the host tserver's client which retries the request against the new master leader. Defined a new wait state `kWaitForLockersMultiple` as with all pggate rpcs which represents that the call is outstanding/being processed at the tserver/docdb side. **//ALTER TABLE ... CONCURRENTLY//** Prior to this revision, `YBCPrepareAlterTableCmd` took `AccessExclusive` locks immaterial of the alter type. The upstream code calculates the locktype that needs to be taken based on the alter type, which wasn't being honored here. Part of the reason might be due to the fact that all of these locks were a no-op back when object locking wasn't supported. This revision refactors the function to take input `lockmode` and lock the tables in that mode, thus //bringing ALTER TABLE concurrency to parity with PostgreSQL//. `lockmode` is computed in `utility.c` ``` * Figure out lock mode, and acquire lock. This also does * basic permissions checks, so that we won't wait for a * lock on (for example) a relation on which we have no * permissions. */ lockmode = AlterTableGetLockLevel(atstmt->cmds); ``` Note that the above analysis of `alter ... concurrently` not being concurrent is true just with table locks enabled, and this revision fixes that behavior. Prior to table locks, ALTER wouldn't wait on DMLs, and that behavior is still preserved when table locking is disabled. **Upgrade / Downgrade section** The new rpcs and usage of the proto messages is protected under the table locks gflag itself which hasn't gone with on by default in any major release yet. So there should not be any issue of upgrade/downgrade safety. **Additional Note** Filed https://github.com/yugabyte/yugabyte-db/issues/31532 to track the discrepancy of observing stale partition hierarchy in YB as compared to PG after the first phase of `ALTER DETACH CONCURRENTLY` commits and before phase 2 finishes. Test Plan: Jenkins Added test cases that assert basic functionality, concurrent `WaitForLockersMultiple` requests, scenarios where `WaitForLockers` times out, master leader loses leadership, etc. Also ported pg isolation test suite for `ALTER DETACH CONCURRENTLY` with some modifications and a follow-up issue. ``` ./yb_build.sh --cxx-test='TEST_F(ObjectLockTest, TestWaitForLockersMultiple) {' ./yb_build.sh --cxx-test='TEST_F(ExternalObjectLockTest, TestWaitForLockers) {' ./yb_build.sh --cxx-test object_lock-test --gtest_filter *WaitForLockersAcrossMasterFailover* ./yb_build.sh --java-test org.yb.pgsql.TestPgRegressIsolationObjectLocking#testPgRegress ``` Reviewers: amitanand, sanketh, pjain, patnaik.balivada, smishra Reviewed By: patnaik.balivada Subscribers: myang, ybase, yql Differential Revision: https://phorge.dev.yugabyte.com/D52015
| Commit: | 50b8ed8 | |
|---|---|---|
| Author: | Basava | |
| Committer: | Basava Kolagani | |
[BACKPORT 2025.2][#31594] DocDB: Table Locks: Fix false deadlock caused by re-use of session level txn. Summary: In one of the itests, we saw create index statements running into false deadlocks. It is due to reuse of the session level transaction. The flow is something as below ``` s1 s2 $create index idx1 on test(v1); - phase1, acquires session object lock on relation test in the end associated with session_txn1 - phase2 $analyze test; - waits on session_txn1 - phase3, releases session object lock on relation test - unblocks ... analyze in progress $create index idx1 on test(v1); - ddl txn tries acquiring conflicting object lock on relation test leads to ddl_txn2 -> analyze_txn ``` The above leads to a deadlock. Since the session level txn has a inherent dependency onto the host txn, we end up with a false cycle ``` session_txn1 -> ddl_txn2 -> analyze_txn -> session_txn1 ``` This is due to the earlier edge `analyze_txn -> session_txn1` not being pruned. The wait-for edges in YB are pruned either when the corresponding subtxn rollsback or the txn itself isn't active anymore. This revision fixes the above false deadlock issue by associating the session object locks from different transactions to different subtxns. In specific, - when acquiring a session level object lock, if one doesn't already exist, bump up the active subtxn and acquire the session object locks against it (stored in `subtxn_with_session_object_locks_`). - when acquiring a session level object lock, if an active subtxn with session locks already exists, acquire them against the same subtxn - on release all session locks, rollback to `subtxn_with_session_object_locks_` and bump the active subtxn id The same session level txn is also used for acquiring session advisory locks, and they need to honored despite the above logic of rolling back specific subtxns. The above logic takes care of this since we increment active subtxn when acquiring a session object lock for the first time, ensuring that all active advisory locks associated with the current subtxn (before the increment) would still remain active. Additionally an `SCHECK` is introduced on the acquire session advisory lock path which ensures that we don't serve any session advisory lock requests when the session level txn is holding active session object locks. This is necessary as release all session object locks rollsback the subtxn. This SCHECK itself is expected to never fail since session advisory locks are user level and cannot be issue in the middle of execution of a DDL (like CREATE INDEX) which use session object locks. **Upgrade/Downgrade safety** Added new field to the proto message which is used for ysql <-> local tserver communication alone. No upgrade/downgrade impact. Additionally, the usage of the feature (table locking) is disabled by default in all existing major releases. Original commit: f37604b92442062f18f1f39e1525e86a67925dc5 / D53287 Test Plan: Jenkins ./yb_build.sh --cxx-test pg_object_locks-test --gtest_filter PgObjectLocksTest.ConsecutiveCreateIndexDontDeadlock Reviewers: amitanand, #db-approvers, hsunder Reviewed By: amitanand, #db-approvers, hsunder Subscribers: svc_phabricator, ybase, yql Differential Revision: https://phorge.dev.yugabyte.com/D53950
| Commit: | 69b7475 | |
|---|---|---|
| Author: | Sanketh I | |
| Committer: | Sanketh I | |
[BACKPORT 2025.2][#29647] YSQL: Avoid master query on PG startup for colocation info Summary: Original commit: c74b3d13b6d3 / D53350 Every PG backend startup queries the master to learn whether its target database is colocated (added a while ago in f0082094c4583989665b72e78180d5874600090e). This diff introduces a tserver-side cache by db oid for this information to avoid this RPC. DB colocation information does not change once the db is created. 1. When multiple backends look up this information at the same time, only one outstanding query is made to the master and the remaining backends use the result of that query (both in success & failure cases). 2. If a query to the master for this info fails, there is no negative cache of the failed result. 3. When a db is dropped, heartbeats to the tserver inform of this event - this is currently used to keep the db oid -> catalog version map accurate and remove entries from it. The same path is used to also keep this cache up to date for dropped dbs. **Upgrade/Rollback safety:** Test Plan: No new test is introduced. A manual test confirmed the behavior for regular hits/misses/drop db. Jenkins tests confirm that no breakage happened. Reviewers: myang, zdrudi Reviewed By: zdrudi Differential Revision: https://phorge.dev.yugabyte.com/D53795
| Commit: | d0d57ad | |
|---|---|---|
| Author: | Hideaki Kimura | |
| Committer: | Hideaki Kimura | |
[BACKPORT 2024.2][#31951] yb-admin: Add a key range to get_table_hash Summary: Backport note: two conflicts, both from newer master code absent on 2024.2. In yb-admin_cli.cc, dropped the `unsafe_release_object_locks_global` command (its args, action, and REGISTER_COMMAND_HIDDEN) -- pre-existing object-locking master code, not part of this change, and ReleaseObjectLocksGlobal does not exist on this branch. Kept this change's DecodeHexPartitionKey helper, the new includes, and the key-range args. The yb-admin-test.cc conflict was this change's new tests landing next to branch-specific context; took the tests as-is. No other changes. get_table_hash hashes a whole table. To narrow a detected inconsistency (e.g. during xCluster verification) down to where the data diverged, allow the scan to be restricted to a logical partition-key sub-range. Add optional start_key / end_key arguments (raw partition keys, hex-encoded on the command line -- the same encoding shown as partition_key_start / partition_key_end by list_tablets). start_key is inclusive, end_key is exclusive; an empty bound means unbounded on that side. The range is logical, so it is cluster-independent: each cluster resolves it to whatever tablets it owns, which is what makes it usable for cross-cluster comparison even when tablet boundaries differ. - DumpTabletDataRequestPB gains start_key and end_key. - tablet::DumpTabletData builds each table's encoded bound as [table prefix][encoded partition key]: the table prefix (cotable_id / colocation_id bytes; empty for a non-colocated table) places the bound in this table's slice of the tablet, and the encoded partition key narrows within it. An empty user bound leaves that side at the iterator's natural table boundary. - yb-admin's client skips tablets that do not overlap the requested range and forwards the bounds unchanged to every overlapping tablet. A key range scopes a single table, so it requires a concrete table_id: combined with a colocation parent id (which hashes every table in the tablet) it would be ambiguous, and is rejected with InvalidArgument. Bad input is rejected up front rather than silently hashing the wrong range: the CLI rejects malformed hex and an inverted range (start_key >= end_key), and the server rejects a bound that is not a 2-byte hash for a hash-partitioned table. Builds on #31952 (D53893, landed), whose per-table scoping this composes with: pass a child colocated table id to hash one colocated table over a key range. For #31951. **Upgrade/Rollback safety:** No persistent or on-disk format change, and no AutoFlag is needed. start_key and end_key are optional fields on DumpTabletDataRequestPB, an on-demand admin RPC used only by `yb-admin get_table_hash`; they are not written to disk, the WAL, or sys.catalog, and are absent from any consensus/replication path. Mixed-version behavior is safe: an old yb-admin sends no bounds, so a new tserver scans the full table (unchanged); a new yb-admin's bounds are ignored by an old tserver, which scans the full table rather than erroring -- run a version-matched yb-admin when using start_key/end_key. No state is persisted, so rollback has nothing to undo. Original commit: 208a0f88102d23f40365f4ba3d2c5005eddee88e / D53900 Test Plan: AdminCliTest.TestGetTableXorHashKeyRange (non-colocated YCQL hash table): explicit empty bounds reproduce the full-table totals, and a complementary 0x8000 split partitions the rows so counts sum and hashes XOR back to the full totals. PgLibPqTest.TestGetTableXorHashColocatedKeyRange (colocated, range-only table): derives real mid-data split keys for id=4 and id=8 from the server's own partitioning (a throwaway non-colocated SPLIT AT VALUES ((4), (8)) table), then -- passing the child colocated table id -- splits the table into three disjoint segments [-inf, key(4)) / [key(4), key(8)) / [key(8), +inf) (the middle one specifies both bounds), verifying exact per-segment row counts (3/4/3) and that the segments recombine (counts sum, hashes XOR) to the full totals. Also asserts that a key range against the colocation parent table id is rejected. Verified locally (debug/clang21): AdminCliTest.TestGetTableXorHashKeyRange passes. Relying on CSI for the full suite (incl. PgLibPqTest.TestGetTableXorHashColocatedKeyRange). Reviewers: jhe Reviewed By: jhe Differential Revision: https://phorge.dev.yugabyte.com/D54293
| Commit: | b4389de | |
|---|---|---|
| Author: | Hideaki Kimura | |
| Committer: | Hideaki Kimura | |
[BACKPORT 2025.2][#31951] yb-admin: Add a key range to get_table_hash Summary: get_table_hash hashes a whole table. To narrow a detected inconsistency (e.g. during xCluster verification) down to where the data diverged, allow the scan to be restricted to a logical partition-key sub-range. Add optional start_key / end_key arguments (raw partition keys, hex-encoded on the command line -- the same encoding shown as partition_key_start / partition_key_end by list_tablets). start_key is inclusive, end_key is exclusive; an empty bound means unbounded on that side. The range is logical, so it is cluster-independent: each cluster resolves it to whatever tablets it owns, which is what makes it usable for cross-cluster comparison even when tablet boundaries differ. - DumpTabletDataRequestPB gains start_key and end_key. - tablet::DumpTabletData builds each table's encoded bound as [table prefix][encoded partition key]: the table prefix (cotable_id / colocation_id bytes; empty for a non-colocated table) places the bound in this table's slice of the tablet, and the encoded partition key narrows within it. An empty user bound leaves that side at the iterator's natural table boundary. - yb-admin's client skips tablets that do not overlap the requested range and forwards the bounds unchanged to every overlapping tablet. A key range scopes a single table, so it requires a concrete table_id: combined with a colocation parent id (which hashes every table in the tablet) it would be ambiguous, and is rejected with InvalidArgument. Bad input is rejected up front rather than silently hashing the wrong range: the CLI rejects malformed hex and an inverted range (start_key >= end_key), and the server rejects a bound that is not a 2-byte hash for a hash-partitioned table. Builds on #31952 (D53893, landed), whose per-table scoping this composes with: pass a child colocated table id to hash one colocated table over a key range. For #31951. **Upgrade/Rollback safety:** No persistent or on-disk format change, and no AutoFlag is needed. start_key and end_key are optional fields on DumpTabletDataRequestPB, an on-demand admin RPC used only by `yb-admin get_table_hash`; they are not written to disk, the WAL, or sys.catalog, and are absent from any consensus/replication path. Mixed-version behavior is safe: an old yb-admin sends no bounds, so a new tserver scans the full table (unchanged); a new yb-admin's bounds are ignored by an old tserver, which scans the full table rather than erroring -- run a version-matched yb-admin when using start_key/end_key. No state is persisted, so rollback has nothing to undo. Original commit: 208a0f88102d23f40365f4ba3d2c5005eddee88e / D53900 Test Plan: AdminCliTest.TestGetTableXorHashKeyRange (non-colocated YCQL hash table): explicit empty bounds reproduce the full-table totals, and a complementary 0x8000 split partitions the rows so counts sum and hashes XOR back to the full totals. PgLibPqTest.TestGetTableXorHashColocatedKeyRange (colocated, range-only table): derives real mid-data split keys for id=4 and id=8 from the server's own partitioning (a throwaway non-colocated SPLIT AT VALUES ((4), (8)) table), then -- passing the child colocated table id -- splits the table into three disjoint segments [-inf, key(4)) / [key(4), key(8)) / [key(8), +inf) (the middle one specifies both bounds), verifying exact per-segment row counts (3/4/3) and that the segments recombine (counts sum, hashes XOR) to the full totals. Also asserts that a key range against the colocation parent table id is rejected. Verified locally (debug/clang21): AdminCliTest.TestGetTableXorHashKeyRange passes. Relying on CSI for the full suite (incl. PgLibPqTest.TestGetTableXorHashColocatedKeyRange). Reviewers: jhe Reviewed By: jhe Differential Revision: https://phorge.dev.yugabyte.com/D54291
| Commit: | 00de1e2 | |
|---|---|---|
| Author: | Hideaki Kimura | |
| Committer: | Hideaki Kimura | |
[BACKPORT 2026.1][#31951] yb-admin: Add a key range to get_table_hash Summary: get_table_hash hashes a whole table. To narrow a detected inconsistency (e.g. during xCluster verification) down to where the data diverged, allow the scan to be restricted to a logical partition-key sub-range. Add optional start_key / end_key arguments (raw partition keys, hex-encoded on the command line -- the same encoding shown as partition_key_start / partition_key_end by list_tablets). start_key is inclusive, end_key is exclusive; an empty bound means unbounded on that side. The range is logical, so it is cluster-independent: each cluster resolves it to whatever tablets it owns, which is what makes it usable for cross-cluster comparison even when tablet boundaries differ. - DumpTabletDataRequestPB gains start_key and end_key. - tablet::DumpTabletData builds each table's encoded bound as [table prefix][encoded partition key]: the table prefix (cotable_id / colocation_id bytes; empty for a non-colocated table) places the bound in this table's slice of the tablet, and the encoded partition key narrows within it. An empty user bound leaves that side at the iterator's natural table boundary. - yb-admin's client skips tablets that do not overlap the requested range and forwards the bounds unchanged to every overlapping tablet. A key range scopes a single table, so it requires a concrete table_id: combined with a colocation parent id (which hashes every table in the tablet) it would be ambiguous, and is rejected with InvalidArgument. Bad input is rejected up front rather than silently hashing the wrong range: the CLI rejects malformed hex and an inverted range (start_key >= end_key), and the server rejects a bound that is not a 2-byte hash for a hash-partitioned table. Builds on #31952 (D53893, landed), whose per-table scoping this composes with: pass a child colocated table id to hash one colocated table over a key range. For #31951. **Upgrade/Rollback safety:** No persistent or on-disk format change, and no AutoFlag is needed. start_key and end_key are optional fields on DumpTabletDataRequestPB, an on-demand admin RPC used only by `yb-admin get_table_hash`; they are not written to disk, the WAL, or sys.catalog, and are absent from any consensus/replication path. Mixed-version behavior is safe: an old yb-admin sends no bounds, so a new tserver scans the full table (unchanged); a new yb-admin's bounds are ignored by an old tserver, which scans the full table rather than erroring -- run a version-matched yb-admin when using start_key/end_key. No state is persisted, so rollback has nothing to undo. Original commit: 208a0f88102d23f40365f4ba3d2c5005eddee88e / D53900 Test Plan: AdminCliTest.TestGetTableXorHashKeyRange (non-colocated YCQL hash table): explicit empty bounds reproduce the full-table totals, and a complementary 0x8000 split partitions the rows so counts sum and hashes XOR back to the full totals. PgLibPqTest.TestGetTableXorHashColocatedKeyRange (colocated, range-only table): derives real mid-data split keys for id=4 and id=8 from the server's own partitioning (a throwaway non-colocated SPLIT AT VALUES ((4), (8)) table), then -- passing the child colocated table id -- splits the table into three disjoint segments [-inf, key(4)) / [key(4), key(8)) / [key(8), +inf) (the middle one specifies both bounds), verifying exact per-segment row counts (3/4/3) and that the segments recombine (counts sum, hashes XOR) to the full totals. Also asserts that a key range against the colocation parent table id is rejected. Verified locally (debug/clang21): AdminCliTest.TestGetTableXorHashKeyRange passes. Relying on CSI for the full suite (incl. PgLibPqTest.TestGetTableXorHashColocatedKeyRange). Reviewers: jhe Reviewed By: jhe Differential Revision: https://phorge.dev.yugabyte.com/D54289
| Commit: | 6c735ec | |
|---|---|---|
| Author: | Hideaki Kimura | |
| Committer: | Hideaki Kimura | |
[BACKPORT 2025.1][#31952] yb-admin: Hash a single colocated table in get_table_hash Summary: Backport note: one conflict, in tablet_dump_helper.cc. Dropped the vector-index skip block (`if (table_info->IsVectorIndex()) continue;`) -- that is pre-existing master code, not part of this change, and `IsVectorIndex` does not exist on this branch. Kept the `target_table_found` assignment that this change adds. No other conflicts. get_table_hash hashes an entire tablet. For a colocated tablet (which hosts multiple colocated tables) the command hashes all of them and the requested table_id is effectively ignored -- so a detected inconsistency cannot be narrowed to the specific colocated table that diverged. Passing any colocated table's id returns the whole-tablet result; that is a bug. Scope the hash by the table_id the command is invoked for: - a colocation parent table id hashes every table in the tablet (for a colocated database, all the colocated tables sharing it) -- the previous whole-tablet behavior, now requested explicitly; - any other table id hashes only that single (colocated or non-colocated) table. - DumpTabletDataRequestPB gains table_id; the yb-admin client always forwards it. - tablet_service derives the scope: an unset or colocation-parent table id hashes all tables, otherwise the single requested table. tablet::DumpTabletData skips colocated tables other than the target and errors if the target is not in the tablet. Per-table iterator scoping (via cotable_id) already existed, so this only adds filtering on top of it. Non-colocated tables are unaffected (one table per tablet). This changes behavior for callers that passed a colocated child table id and relied on getting the whole-tablet hash. That behavior was a bug and the tool is new, so we fix it directly (no opt-in flag) and backport to release branches. To request a whole-tablet hash, pass the colocation parent table id, visible in list_tables when system tables are included. For #31952. Prerequisite for the key-range work in #31951. **Upgrade/Rollback safety:** No persistent or on-disk format change, and no AutoFlag is needed. table_id is an optional field on DumpTabletDataRequestPB, an on-demand admin RPC used only by `yb-admin get_table_hash`; it is not written to disk, the WAL, or sys.catalog, and is absent from any consensus/replication path. Mixed-version behavior is safe: an old yb-admin sends no table_id, so a new tserver hashes the whole tablet (unchanged); a new yb-admin's table_id is ignored by an old tserver, which also hashes the whole tablet -- run a version-matched yb-admin to scope to a single colocated table. No state is persisted, so rollback has nothing to undo. Original commit: 048f6f127587cda08991d74ebe0e54f7d30c9aa7 / D53893 Test Plan: AdminCliTestWithYSQL.TestGetTableXorHashColocated: a colocated database with two tables; a child table id hashes just that table (distinct non-zero hashes and correct per-table row counts), and the colocation parent table id hashes the whole tablet (row counts sum and per-table hashes XOR back to the whole-tablet hash). Relying on CSI to build and run the test. --- _automated · Claude Code (Opus 4.8)_ Reviewers: jhe, #db-approvers Reviewed By: jhe, #db-approvers Subscribers: svc_phabricator Differential Revision: https://phorge.dev.yugabyte.com/D54193
| Commit: | b6bbc7a | |
|---|---|---|
| Author: | Craig Soules | |
| Committer: | GitHub | |
[#30578] YSQL: Reset auto-analyze mutation counts after a manual ANALYZE (#31849) Reset auto-analyze mutation counters after user-initiated ANALYZE ## Problem User-initiated ANALYZE does not reset the auto-analyze service's accumulated mutation count. The next periodic tick can therefore trigger an auto-analyze on a table the user just analyzed, wasting work. ## Solution Add ResetAutoAnalyzeMutationCounters RPC (pg_client.proto / pg_client_service) called from do_analyze_rel after a full ANALYZE. The RPC resets the YCQL service table mutations to 0 for the analyzed table. Reset is gated to match PostgreSQL's semantics for updating its changes_since_analyze counters (fires only when no column list given) and is suppressed for auto-analyze service's internal connections and other internal backends. Auto analyze continues to reset mutations using the existing logic to subtract mutations on its side. Mutations are stored in a separate YCQL table so the mutations update is not transactional. A failed manual ANALYZE can still reset counters to 0. Fixing this is tracked in #32081. ## Race handling Extract mutation-update logic into helper functions in pg_auto_analyze_table that use conditional YCQL writes for better reuse. - ResetPgAutoAnalyzeMutationCounts: sets mutations to 0 (IF EXISTS) - SubtractPgAutoAnalyzeMutationCounts: subtracts snapshot mutations with clamping. Emits two conditional writes per table — one sets to 0 if current < snapshot, another subtracts if current >= snapshot. This prevents the count from going negative when a manual-ANALYZE reset races with auto-analyze post-ANALYZE mutation subtraction. UpdateTableMutationsAfterAnalyze refactored to use SubtractPgAutoAnalyzeMutationCounts instead of building operations directly. ## Wire-format changes New RPC: PgClientService.ResetAutoAnalyzeMutationCounters (pg_client.proto). Additive only — no existing field numbers or messages altered. Upgrade/rollback safety: New RPC is between PG and local tserver so no upgrade/rollback issues. ## Test plan - PgAutoAnalyzeTest.ManualAnalyzeResetsMutationCount: covers ANALYZE, ANALYZE(col), VACUUM ANALYZE, VACUUM ANALYZE(col), asserting reset only when no column list provided. - PgAutoAnalyzeTest.InternalAnalyzeDoesNotResetMutationCount: confirms internal connections do not trigger reset. - PgAutoAnalyzeTest.ManualAnalyzePartitionedTableResetsPartitionMutationCounts: exercises the partitioned-table path. - Existing PgAutoAnalyzeTest cases continue to pass.
| Commit: | 2495406 | |
|---|---|---|
| Author: | Hideaki Kimura | |
| Committer: | Hideaki Kimura | |
[BACKPORT 2024.2][#31952] yb-admin: Hash a single colocated table in get_table_hash Summary: Backport note: three conflicts, all resolved to keep only what this change adds. - tablet_dump_helper.cc: dropped the vector-index skip block (pre-existing master code; `IsVectorIndex` does not exist on this branch); kept the `target_table_found` assignment this change adds. - tablet_service.cc: added only `#include "yb/common/colocated_util.h"` (for IsColocationParentTableId). The adjacent `pg_types.h` include in the master hunk was pre-existing master context, absent here and not needed by this change. - yb-admin-test.cc: the conflict bundled in the unrelated, pre-existing TestPartitionRangeFormat test (not on this branch) -- excluded it. Added the AdminCliTestWithYSQL fixture (pre-existing on master, absent here and required by the new test), the GetColocationParentTableId helper, and TestGetTableXorHashColocated. get_table_hash hashes an entire tablet. For a colocated tablet (which hosts multiple colocated tables) the command hashes all of them and the requested table_id is effectively ignored -- so a detected inconsistency cannot be narrowed to the specific colocated table that diverged. Passing any colocated table's id returns the whole-tablet result; that is a bug. Scope the hash by the table_id the command is invoked for: - a colocation parent table id hashes every table in the tablet (for a colocated database, all the colocated tables sharing it) -- the previous whole-tablet behavior, now requested explicitly; - any other table id hashes only that single (colocated or non-colocated) table. - DumpTabletDataRequestPB gains table_id; the yb-admin client always forwards it. - tablet_service derives the scope: an unset or colocation-parent table id hashes all tables, otherwise the single requested table. tablet::DumpTabletData skips colocated tables other than the target and errors if the target is not in the tablet. Per-table iterator scoping (via cotable_id) already existed, so this only adds filtering on top of it. Non-colocated tables are unaffected (one table per tablet). This changes behavior for callers that passed a colocated child table id and relied on getting the whole-tablet hash. That behavior was a bug and the tool is new, so we fix it directly (no opt-in flag) and backport to release branches. To request a whole-tablet hash, pass the colocation parent table id, visible in list_tables when system tables are included. For #31952. Prerequisite for the key-range work in #31951. **Upgrade/Rollback safety:** No persistent or on-disk format change, and no AutoFlag is needed. table_id is an optional field on DumpTabletDataRequestPB, an on-demand admin RPC used only by `yb-admin get_table_hash`; it is not written to disk, the WAL, or sys.catalog, and is absent from any consensus/replication path. Mixed-version behavior is safe: an old yb-admin sends no table_id, so a new tserver hashes the whole tablet (unchanged); a new yb-admin's table_id is ignored by an old tserver, which also hashes the whole tablet -- run a version-matched yb-admin to scope to a single colocated table. No state is persisted, so rollback has nothing to undo. Original commit: 048f6f127587cda08991d74ebe0e54f7d30c9aa7 / D53893 Test Plan: AdminCliTestWithYSQL.TestGetTableXorHashColocated: a colocated database with two tables; a child table id hashes just that table (distinct non-zero hashes and correct per-table row counts), and the colocation parent table id hashes the whole tablet (row counts sum and per-table hashes XOR back to the whole-tablet hash). Relying on CSI to build and run the test. --- _automated · Claude Code (Opus 4.8)_ Reviewers: jhe Reviewed By: jhe Differential Revision: https://phorge.dev.yugabyte.com/D54194
| Commit: | 208a0f8 | |
|---|---|---|
| Author: | Hideaki Kimura | |
| Committer: | Hideaki Kimura | |
[#31951] yb-admin: Add a key range to get_table_hash Summary: get_table_hash hashes a whole table. To narrow a detected inconsistency (e.g. during xCluster verification) down to where the data diverged, allow the scan to be restricted to a logical partition-key sub-range. Add optional start_key / end_key arguments (raw partition keys, hex-encoded on the command line -- the same encoding shown as partition_key_start / partition_key_end by list_tablets). start_key is inclusive, end_key is exclusive; an empty bound means unbounded on that side. The range is logical, so it is cluster-independent: each cluster resolves it to whatever tablets it owns, which is what makes it usable for cross-cluster comparison even when tablet boundaries differ. - DumpTabletDataRequestPB gains start_key and end_key. - tablet::DumpTabletData builds each table's encoded bound as [table prefix][encoded partition key]: the table prefix (cotable_id / colocation_id bytes; empty for a non-colocated table) places the bound in this table's slice of the tablet, and the encoded partition key narrows within it. An empty user bound leaves that side at the iterator's natural table boundary. - yb-admin's client skips tablets that do not overlap the requested range and forwards the bounds unchanged to every overlapping tablet. A key range scopes a single table, so it requires a concrete table_id: combined with a colocation parent id (which hashes every table in the tablet) it would be ambiguous, and is rejected with InvalidArgument. Bad input is rejected up front rather than silently hashing the wrong range: the CLI rejects malformed hex and an inverted range (start_key >= end_key), and the server rejects a bound that is not a 2-byte hash for a hash-partitioned table. Builds on #31952 (D53893, landed), whose per-table scoping this composes with: pass a child colocated table id to hash one colocated table over a key range. For #31951. **Upgrade/Rollback safety:** No persistent or on-disk format change, and no AutoFlag is needed. start_key and end_key are optional fields on DumpTabletDataRequestPB, an on-demand admin RPC used only by `yb-admin get_table_hash`; they are not written to disk, the WAL, or sys.catalog, and are absent from any consensus/replication path. Mixed-version behavior is safe: an old yb-admin sends no bounds, so a new tserver scans the full table (unchanged); a new yb-admin's bounds are ignored by an old tserver, which scans the full table rather than erroring -- run a version-matched yb-admin when using start_key/end_key. No state is persisted, so rollback has nothing to undo. Test Plan: AdminCliTest.TestGetTableXorHashKeyRange (non-colocated YCQL hash table): explicit empty bounds reproduce the full-table totals, and a complementary 0x8000 split partitions the rows so counts sum and hashes XOR back to the full totals. PgLibPqTest.TestGetTableXorHashColocatedKeyRange (colocated, range-only table): derives real mid-data split keys for id=4 and id=8 from the server's own partitioning (a throwaway non-colocated SPLIT AT VALUES ((4), (8)) table), then -- passing the child colocated table id -- splits the table into three disjoint segments [-inf, key(4)) / [key(4), key(8)) / [key(8), +inf) (the middle one specifies both bounds), verifying exact per-segment row counts (3/4/3) and that the segments recombine (counts sum, hashes XOR) to the full totals. Also asserts that a key range against the colocation parent table id is rejected. Verified locally (debug/clang21): AdminCliTest.TestGetTableXorHashKeyRange passes. Relying on CSI for the full suite (incl. PgLibPqTest.TestGetTableXorHashColocatedKeyRange). Reviewers: jhe Reviewed By: jhe Subscribers: svc_phabricator, yql, ybase Differential Revision: https://phorge.dev.yugabyte.com/D53900
| Commit: | 24f9c71 | |
|---|---|---|
| Author: | Hideaki Kimura | |
| Committer: | Hideaki Kimura | |
[BACKPORT 2025.2][#31952] yb-admin: Hash a single colocated table in get_table_hash Summary: Clean backport, no conflicts. get_table_hash hashes an entire tablet. For a colocated tablet (which hosts multiple colocated tables) the command hashes all of them and the requested table_id is effectively ignored -- so a detected inconsistency cannot be narrowed to the specific colocated table that diverged. Passing any colocated table's id returns the whole-tablet result; that is a bug. Scope the hash by the table_id the command is invoked for: - a colocation parent table id hashes every table in the tablet (for a colocated database, all the colocated tables sharing it) -- the previous whole-tablet behavior, now requested explicitly; - any other table id hashes only that single (colocated or non-colocated) table. - DumpTabletDataRequestPB gains table_id; the yb-admin client always forwards it. - tablet_service derives the scope: an unset or colocation-parent table id hashes all tables, otherwise the single requested table. tablet::DumpTabletData skips colocated tables other than the target and errors if the target is not in the tablet. Per-table iterator scoping (via cotable_id) already existed, so this only adds filtering on top of it. Non-colocated tables are unaffected (one table per tablet). This changes behavior for callers that passed a colocated child table id and relied on getting the whole-tablet hash. That behavior was a bug and the tool is new, so we fix it directly (no opt-in flag) and backport to release branches. To request a whole-tablet hash, pass the colocation parent table id, visible in list_tables when system tables are included. For #31952. Prerequisite for the key-range work in #31951. **Upgrade/Rollback safety:** No persistent or on-disk format change, and no AutoFlag is needed. table_id is an optional field on DumpTabletDataRequestPB, an on-demand admin RPC used only by `yb-admin get_table_hash`; it is not written to disk, the WAL, or sys.catalog, and is absent from any consensus/replication path. Mixed-version behavior is safe: an old yb-admin sends no table_id, so a new tserver hashes the whole tablet (unchanged); a new yb-admin's table_id is ignored by an old tserver, which also hashes the whole tablet -- run a version-matched yb-admin to scope to a single colocated table. No state is persisted, so rollback has nothing to undo. Original commit: 048f6f127587cda08991d74ebe0e54f7d30c9aa7 / D53893 Test Plan: AdminCliTestWithYSQL.TestGetTableXorHashColocated: a colocated database with two tables; a child table id hashes just that table (distinct non-zero hashes and correct per-table row counts), and the colocation parent table id hashes the whole tablet (row counts sum and per-table hashes XOR back to the whole-tablet hash). Relying on CSI to build and run the test. --- _automated · Claude Code (Opus 4.8)_ Reviewers: jhe Reviewed By: jhe Differential Revision: https://phorge.dev.yugabyte.com/D54192
| Commit: | d9de934 | |
|---|---|---|
| Author: | Hideaki Kimura | |
| Committer: | Hideaki Kimura | |
[BACKPORT 2026.1][#31952] yb-admin: Hash a single colocated table in get_table_hash Summary: Clean backport, no conflicts. get_table_hash hashes an entire tablet. For a colocated tablet (which hosts multiple colocated tables) the command hashes all of them and the requested table_id is effectively ignored -- so a detected inconsistency cannot be narrowed to the specific colocated table that diverged. Passing any colocated table's id returns the whole-tablet result; that is a bug. Scope the hash by the table_id the command is invoked for: - a colocation parent table id hashes every table in the tablet (for a colocated database, all the colocated tables sharing it) -- the previous whole-tablet behavior, now requested explicitly; - any other table id hashes only that single (colocated or non-colocated) table. - DumpTabletDataRequestPB gains table_id; the yb-admin client always forwards it. - tablet_service derives the scope: an unset or colocation-parent table id hashes all tables, otherwise the single requested table. tablet::DumpTabletData skips colocated tables other than the target and errors if the target is not in the tablet. Per-table iterator scoping (via cotable_id) already existed, so this only adds filtering on top of it. Non-colocated tables are unaffected (one table per tablet). This changes behavior for callers that passed a colocated child table id and relied on getting the whole-tablet hash. That behavior was a bug and the tool is new, so we fix it directly (no opt-in flag) and backport to release branches. To request a whole-tablet hash, pass the colocation parent table id, visible in list_tables when system tables are included. For #31952. Prerequisite for the key-range work in #31951. **Upgrade/Rollback safety:** No persistent or on-disk format change, and no AutoFlag is needed. table_id is an optional field on DumpTabletDataRequestPB, an on-demand admin RPC used only by `yb-admin get_table_hash`; it is not written to disk, the WAL, or sys.catalog, and is absent from any consensus/replication path. Mixed-version behavior is safe: an old yb-admin sends no table_id, so a new tserver hashes the whole tablet (unchanged); a new yb-admin's table_id is ignored by an old tserver, which also hashes the whole tablet -- run a version-matched yb-admin to scope to a single colocated table. No state is persisted, so rollback has nothing to undo. Original commit: 048f6f127587cda08991d74ebe0e54f7d30c9aa7 / D53893 Test Plan: AdminCliTestWithYSQL.TestGetTableXorHashColocated: a colocated database with two tables; a child table id hashes just that table (distinct non-zero hashes and correct per-table row counts), and the colocation parent table id hashes the whole tablet (row counts sum and per-table hashes XOR back to the whole-tablet hash). Relying on CSI to build and run the test. --- _automated · Claude Code (Opus 4.8)_ Reviewers: jhe Reviewed By: jhe Differential Revision: https://phorge.dev.yugabyte.com/D54191
| Commit: | 54c3d4c | |
|---|---|---|
| Author: | Timur Yusupov | |
| Committer: | Timur Yusupov | |
[#31930,#31241] docdb: Fix RBS vs ConfigChange races Summary: There is a possibility of RBS vs ConfigChange races, for example: 1. Raft config for the tablet has nodes A (leader), B, C. 2. D is added to Raft config, CHANGE_CONFIG operation is committed and applied on A, B, C 3. RBS A -> D started 4. D is removed from Raft config, CHANGE_CONFIG operation is committed and applied on leader 5. RBS A -> D downloads WAL and completed, D has the latest committed Raft config **Expected result:** orphaned tablet replica on D should be deleted. **Actual result:** we have an orphaned tablet replica (lagging follower) on D that is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader. In order to fix that, behaviour inside `MasterHeartbeatServiceImpl::ProcessTabletReport` is changed to delete a tablet replica which is no longer part of the committed Raft config and Raft config that added that replica is no longer pending (either committed or aborted). The latter condition avoids deleting the newly RBSed tablet replica that is being added to the tablet Raft group. The following logic is implemented to support that: 1. Once tablet leader decides to start RBS replica on another tserver, it will include the current pending Raft config op id (both term and index) into `StartRemoteBootstrapRequestPB` or empty op id when no config change is pending. 2. The bootstrapping replica persists this op id in its consensus metadata as `pending_config_op_id_from_rbs`. It is cleared once the replica's last committed op id either advances to a higher term, or its index reaches/passes the stored pending op id's index - i.e. once the original `CHANGE_CONFIG` operation can no longer be pending (it has either committed or been aborted). 3. `ReportedTabletPB::pending_config_op_id` is added to tserver->master heartbeats. Its value is whichever is set: the replica's currently active pending config op id (a config change in progress on the replica itself) or the pending_config_op_id_from_rbs from (2). 4. When master leader receives a tablet report from a replica that is *not* in the committed Raft config last known by the master, the master triggers `DeleteTabletRequestPB` to that replica if both: a. The reported committed Raft config op id index is <= the committed Raft config op id index last known by master for this tablet. b. Either the report carries no pending op id (empty/missing), or the master can prove it is no longer pending: if the pending op id's term is strictly less than the `current_term` in the master's last-known committed consensus state for this tablet, that `CHANGE_CONFIG` must have already been aborted or committed, and condition (4.a) alone is sufficient. 5. TServer will include tablet in next heartbeat in case DeleteTablet failed due to stale cas_config_opid_index_less_or_equal (this scenario is covered by TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart). Also update master-side of CloneTablet operation to seed the target tablet's committed_consensus_state peers on the master from the source tablet's config so that when the cloned replicas heartbeat back, `ProcessTabletReportBatch` sees them as part of the expected Raft config and does not tombstone them. Added several unit-tests for other RBS vs ConfigChange race scenarios. They are also fixed by the implemented change. Also renamed `RaftConfigPB.opid_index` to `committed_op_index` in order to reflect the actual purpose of this field. **Upgrade/Rollback safety:** New logic at master-side is gated by a new `use_tablet_report_pending_config_op_id` auto flag. Until the whole cluster is fully upgraded, master won't rely on newly added `ReportedTabletPB::pending_config_op_id` field. Test Plan: Run the following tests for asan/tsan/debug/relelase for 20 iterations each: - TabletSplitITest.SplitWithParentTabletRbsFromFollower - TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRace - TabletReplacementITest.TombstoneEvictedReplicaWithRbsAndConfigChangeRaceAndMasterRestart - TabletReplacementITest.TombstoneEvictedReplicaAfterAbortedAddServer - covers https://github.com/yugabyte/yugabyte-db/issues/31241 - TabletReplacementITest.DontDeleteNewReplicaInPendingConfig - TabletReplacementITest.DontDeleteNewReplicaInPendingConfigAfterRbsFromFollowerRf5 Reviewers: zdrudi Reviewed By: zdrudi Subscribers: ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D52759
| Commit: | 048f6f1 | |
|---|---|---|
| Author: | Hideaki Kimura | |
| Committer: | Hideaki Kimura | |
[#31952] yb-admin: Hash a single colocated table in get_table_hash Summary: get_table_hash hashes an entire tablet. For a colocated tablet (which hosts multiple colocated tables) the command hashes all of them and the requested table_id is effectively ignored -- so a detected inconsistency cannot be narrowed to the specific colocated table that diverged. Passing any colocated table's id returns the whole-tablet result; that is a bug. Scope the hash by the table_id the command is invoked for: - a colocation parent table id hashes every table in the tablet (for a colocated database, all the colocated tables sharing it) -- the previous whole-tablet behavior, now requested explicitly; - any other table id hashes only that single (colocated or non-colocated) table. - DumpTabletDataRequestPB gains table_id; the yb-admin client always forwards it. - tablet_service derives the scope: an unset or colocation-parent table id hashes all tables, otherwise the single requested table. tablet::DumpTabletData skips colocated tables other than the target and errors if the target is not in the tablet. Per-table iterator scoping (via cotable_id) already existed, so this only adds filtering on top of it. Non-colocated tables are unaffected (one table per tablet). This changes behavior for callers that passed a colocated child table id and relied on getting the whole-tablet hash. That behavior was a bug and the tool is new, so we fix it directly (no opt-in flag) and backport to release branches. To request a whole-tablet hash, pass the colocation parent table id, visible in list_tables when system tables are included. For #31952. Prerequisite for the key-range work in #31951. **Upgrade/Rollback safety:** No persistent or on-disk format change, and no AutoFlag is needed. table_id is an optional field on DumpTabletDataRequestPB, an on-demand admin RPC used only by `yb-admin get_table_hash`; it is not written to disk, the WAL, or sys.catalog, and is absent from any consensus/replication path. Mixed-version behavior is safe: an old yb-admin sends no table_id, so a new tserver hashes the whole tablet (unchanged); a new yb-admin's table_id is ignored by an old tserver, which also hashes the whole tablet -- run a version-matched yb-admin to scope to a single colocated table. No state is persisted, so rollback has nothing to undo. Test Plan: AdminCliTestWithYSQL.TestGetTableXorHashColocated: a colocated database with two tables; a child table id hashes just that table (distinct non-zero hashes and correct per-table row counts), and the colocation parent table id hashes the whole tablet (row counts sum and per-table hashes XOR back to the whole-tablet hash). Local build/test is blocked by an unrelated documentdb postgres build failure in the dev environment; relying on CSI to build and run the test. Reviewers: jhe Reviewed By: jhe Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D53893
| Commit: | 79ba8b5 | |
|---|---|---|
| Author: | eg | |
| Committer: | GitHub | |
[BACKPORT 2026.1][#26918] YSQL: Stop index backfill when the CREATE INDEX session is terminated (#31378) (#32003) ## Summary Add DdlRequesterLivenessTask, a master-side task that polls the transaction status of the DDL transaction held open by the CREATE INDEX CONCURRENTLY backend. If the transaction is aborted (e.g. because the backend was killed via pg_terminate_backend), the task calls BackfillTable::Abort() to stop the in-progress backfill. Original commit: b51a5c9524e153fa7d7a9f47f625c03798fa03d0 / #31378 ## Test plan - PgIndexBackfillCancellationTest.BackfillStopsAfterBackendKill -- asserts that no new backfill RPCs are issued after the backend is killed. - PgIndexBackfillCancellationWithoutFixTest.BackfillContinuesAfterBackendKill -- asserts the old behavior (backfill continues) when the liveness monitor is disabled, serving as a regression baseline. - PgIndexBackfillCancellationEarlyKillTest.BackfillStopsAfterEarlyBackendKill -- same as above but the backend is killed before backfill starts. ## Upgrade / Rollback Safety Upgrade safety: Safe. Rollback safety: Safe. No special procedures are required. [CSI](<https://csiweb.dev.yugabyte.com/pull/31378/>) Jira: [DB-16358](https://yugabyte.atlassian.net/browse/DB-16358) [DB-16358]: https://yugabyte.atlassian.net/browse/DB-16358?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [DB-16358]: https://yugabyte.atlassian.net/browse/DB-16358?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
| Commit: | b0bfc62 | |
|---|---|---|
| Author: | Basava | |
| Committer: | Basava | |
[BACKPORT 2026.1][#31594] DocDB: Table Locks: Fix false deadlock caused by re-use of session level txn. Summary: In one of the itests, we saw create index statements running into false deadlocks. It is due to reuse of the session level transaction. The flow is something as below ``` s1 s2 $create index idx1 on test(v1); - phase1, acquires session object lock on relation test in the end associated with session_txn1 - phase2 $analyze test; - waits on session_txn1 - phase3, releases session object lock on relation test - unblocks ... analyze in progress $create index idx1 on test(v1); - ddl txn tries acquiring conflicting object lock on relation test leads to ddl_txn2 -> analyze_txn ``` The above leads to a deadlock. Since the session level txn has a inherent dependency onto the host txn, we end up with a false cycle ``` session_txn1 -> ddl_txn2 -> analyze_txn -> session_txn1 ``` This is due to the earlier edge `analyze_txn -> session_txn1` not being pruned. The wait-for edges in YB are pruned either when the corresponding subtxn rollsback or the txn itself isn't active anymore. This revision fixes the above false deadlock issue by associating the session object locks from different transactions to different subtxns. In specific, - when acquiring a session level object lock, if one doesn't already exist, bump up the active subtxn and acquire the session object locks against it (stored in `subtxn_with_session_object_locks_`). - when acquiring a session level object lock, if an active subtxn with session locks already exists, acquire them against the same subtxn - on release all session locks, rollback to `subtxn_with_session_object_locks_` and bump the active subtxn id The same session level txn is also used for acquiring session advisory locks, and they need to honored despite the above logic of rolling back specific subtxns. The above logic takes care of this since we increment active subtxn when acquiring a session object lock for the first time, ensuring that all active advisory locks associated with the current subtxn (before the increment) would still remain active. Additionally an `SCHECK` is introduced on the acquire session advisory lock path which ensures that we don't serve any session advisory lock requests when the session level txn is holding active session object locks. This is necessary as release all session object locks rollsback the subtxn. This SCHECK itself is expected to never fail since session advisory locks are user level and cannot be issue in the middle of execution of a DDL (like CREATE INDEX) which use session object locks. **Upgrade/Downgrade safety** Added new field to the proto message which is used for ysql <-> local tserver communication alone. No upgrade/downgrade impact. Additionally, the usage of the feature (table locking) is disabled by default in all existing major releases. Original commit: f37604b92442062f18f1f39e1525e86a67925dc5 / D53287 Test Plan: Jenkins ./yb_build.sh --cxx-test pg_object_locks-test --gtest_filter PgObjectLocksTest.ConsecutiveCreateIndexDontDeadlock Reviewers: amitanand Reviewed By: amitanand Subscribers: ybase, yql Differential Revision: https://phorge.dev.yugabyte.com/D53949
| Commit: | 4f8e1b3 | |
|---|---|---|
| Author: | William Wang | |
| Committer: | William Wang | |
[#27168] YSQL: yb-admin UpgradeYsql should pick tserver closest to master Summary: Introduced a new RPC endpoint for master admin (`ClientUpgradeYsql`) and changed client's `UpgradeYsql` flow from: client picks the first tserver that is available -> calls the tserver RPC directly to: client calls master's `ClientUpgradeYsql` RPC with the parameters of the server RPC -> master finds the closest tserver and calls it on client's behalf -> master returns the response of the tserver RPC call to client. The tserver RPC call that master makes is similar to the existing implementation of the `CollectViaRpc` method in `master_call_home.cc`. Unit tests have been added for the newly added master admin RPC. **Upgrade/Rollback safety:** A new yb-admin RPC has been added. New yb-admin client now relies on this RPC when calling `UpgradeYsql`. In the (unlikely) event of a new yb-admin client trying to call this RPC on an old master, the upgrade function will fail. Upgrades/Rollbacks should be safe otherwise. Test Plan: `./yb_build.sh release --cxx-test master-test --gtest_filter 'MasterTestClientUpgradeYsql.*'` Reviewers: kfranz Reviewed By: kfranz Subscribers: ybase, yql Differential Revision: https://phorge.dev.yugabyte.com/D53952
| Commit: | 390ba45 | |
|---|---|---|
| Author: | Anton Rybochkin | |
| Committer: | Anton Rybochkin | |
[#31958] docdb: Vector Index: Skip reverse mapping insertion during backfill Summary: Currently, vector index reverse mapping entries are inserted for all existing vectors whenever a new vector index is created. This approach inserts the same data repeatedly each time a new vector index is created for the same vector column, which is incorrect. Instead, the indexed table should own the reverse mapping and manage its insertion and deletion. Therefore, reverse mapping population should not happen during vector index backfill. This change addresses the backfill aspect and implements the logic to skip reverse mapping population while a vector index is being backfilled. The logic is currently disabled until the remaining parts of the reverse mapping ownership logic are implemented (https://github.com/yugabyte/yugabyte-db/issues/31886). For backward compatibility, `PgVectorIdxOptionsPB` is extended with a new field, `skip_reverse_mapping_backfill`, which is automatically unset in older releases because the field is not present there. This logic is required to ensure that the old approach is used when vector index backfill started before the upgrade (so some reverse mapping entries may have already been added) but had not yet completed by the time of the upgrade. **Upgrade/Rollback safety:** The change is backward compatible, and the absence of the new field is treated as the old behavior -- this is actually required to allow an in-progress backfill started before the upgrade to complete using the old approach, ensuring that no vectors are lost. Test Plan: yb_build.sh --cxx-test='TEST_F(PgVectorIndexUtilTest, BackfillSkipsReverseMapping)' yb_build.sh --cxx-test='TEST_F(PgVectorIndexUtilTest, BackfillWritesReverseMapping)' Reviewers: sergei, zdrudi Reviewed By: sergei, zdrudi Subscribers: ybase, yql Differential Revision: https://phorge.dev.yugabyte.com/D53921
| Commit: | 3312a18 | |
|---|---|---|
| Author: | Sanketh I | |
| Committer: | Sanketh I | |
[BACKPORT 2026.1][#29647] YSQL: Avoid master query on PG startup for colocation info Summary: Original commit: c74b3d13b6d3 / D53350 Every PG backend startup queries the master to learn whether its target database is colocated (added a while ago in f0082094c4583989665b72e78180d5874600090e). This diff introduces a tserver-side cache by db oid for this information to avoid this RPC. DB colocation information does not change once the db is created. 1. When multiple backends look up this information at the same time, only one outstanding query is made to the master and the remaining backends use the result of that query (both in success & failure cases). 2. If a query to the master for this info fails, there is no negative cache of the failed result. 3. When a db is dropped, heartbeats to the tserver inform of this event - this is currently used to keep the db oid -> catalog version map accurate and remove entries from it. The same path is used to also keep this cache up to date for dropped dbs. **Upgrade/Rollback safety:** A new PG -> local tserver is introduced. PG and tserver are upgraded at the same time, so this should be safe. Test Plan: No new test is introduced. A manual test confirmed the behavior for regular hits/misses/drop db. Jenkins tests confirm that no breakage happened. Reviewers: myang, zdrudi Reviewed By: zdrudi Subscribers: svc_phabricator, ybase, yql Differential Revision: https://phorge.dev.yugabyte.com/D53794
| Commit: | b51a5c9 | |
|---|---|---|
| Author: | eg | |
| Committer: | GitHub | |
[#26918] YSQL: Stop index backfill when the CREATE INDEX session is terminated (#31378) ## Summary Add DdlRequesterLivenessTask, a master-side task that polls the transaction status of the DDL transaction held open by the CREATE INDEX CONCURRENTLY backend. If the transaction is aborted (e.g. because the backend was killed via pg_terminate_backend), the task calls BackfillTable::Abort() to stop the in-progress backfill. ## Test plan - PgIndexBackfillCancellationTest.BackfillStopsAfterBackendKill -- asserts that no new backfill RPCs are issued after the backend is killed. - PgIndexBackfillCancellationWithoutFixTest.BackfillContinuesAfterBackendKill -- asserts the old behavior (backfill continues) when the liveness monitor is disabled, serving as a regression baseline. - PgIndexBackfillCancellationEarlyKillTest.BackfillStopsAfterEarlyBackendKill -- same as above but the backend is killed before backfill starts. ## Upgrade / Rollback Safety Upgrade safety: Safe. Rollback safety: Safe. No special procedures are required. [CSI](<https://csiweb.dev.yugabyte.com/pull/31378/>) Jira: [DB-16358](https://yugabyte.atlassian.net/browse/DB-16358) [DB-16358]: https://yugabyte.atlassian.net/browse/DB-16358?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
| Commit: | e093cc2 | |
|---|---|---|
| Author: | jhe | |
| Committer: | jhe | |
[BACKPORT 2025.2][#31402] docdb: Use TabletInvoker for async writes Summary: Make pipelined async writes survive a leader stepdown when the writes were committed before the stepdown. - `WaitForAsyncWriteRpc` now uses `TabletInvoker`. On `NOT_THE_LEADER` errors from the original leader, it will retry on the new leader - `TabletPeer::VerifyAsyncWriteCompletion` runs on the new leader when the OpId isn't in its in-flight tracker (ie the write went to the previous leader). It checks `op_id.index < first_index_of_current_term_` to decide whether the previous-term entry was committed - If yes, then we can proceed - if no, then we return Aborted and abort the transaction. - For now `VerifyAsyncWriteCompletion` only handles writes from the current and previous term; if the term difference is > 2, then we also abort the transaction. If this is needed in the future, we can do a log lookup to verify the term and index. Also removing usage of `leader_term` for async writes, since this was only used to abort transactions on leadership changes before. **Upgrade/Rollback safety:** The feature is gated by gFlag `ysql_enable_write_pipelining`. The new `ReadRequestPB.pending_async_write_op_id` field is optional and defaults to unset on older servers, which fall back to the previous (no verification) behavior. Original commit: a305ddace1b2991fc55a8809b25935cc5bd455c4 / D51088 Test Plan: async_writes-test Reviewers: hsunder, sergei Reviewed By: hsunder Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D53784
| Commit: | 628acc5 | |
|---|---|---|
| Author: | jhe | |
| Committer: | jhe | |
[BACKPORT 2026.1][#31402] docdb: Use TabletInvoker for async writes Summary: Make pipelined async writes survive a leader stepdown when the writes were committed before the stepdown. - `WaitForAsyncWriteRpc` now uses `TabletInvoker`. On `NOT_THE_LEADER` errors from the original leader, it will retry on the new leader - `TabletPeer::VerifyAsyncWriteCompletion` runs on the new leader when the OpId isn't in its in-flight tracker (ie the write went to the previous leader). It checks `op_id.index < first_index_of_current_term_` to decide whether the previous-term entry was committed - If yes, then we can proceed - if no, then we return Aborted and abort the transaction. - For now `VerifyAsyncWriteCompletion` only handles writes from the current and previous term; if the term difference is > 2, then we also abort the transaction. If this is needed in the future, we can do a log lookup to verify the term and index. Also removing usage of `leader_term` for async writes, since this was only used to abort transactions on leadership changes before. **Upgrade/Rollback safety:** The feature is gated by gFlag `ysql_enable_write_pipelining`. The new `ReadRequestPB.pending_async_write_op_id` field is optional and defaults to unset on older servers, which fall back to the previous (no verification) behavior. Original commit: a305ddace1b2991fc55a8809b25935cc5bd455c4 / D51088 Test Plan: async_writes-test Reviewers: hsunder, sergei Reviewed By: hsunder Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D53782
| Commit: | f37604b | |
|---|---|---|
| Author: | Basava | |
| Committer: | Basava | |
[#31594] DocDB: Table Locks: Fix false deadlock caused by re-use of session level txn. Summary: In one of the itests, we saw create index statements running into false deadlocks. It is due to reuse of the session level transaction. The flow is something as below ``` s1 s2 $create index idx1 on test(v1); - phase1, acquires session object lock on relation test in the end associated with session_txn1 - phase2 $analyze test; - waits on session_txn1 - phase3, releases session object lock on relation test - unblocks ... analyze in progress $create index idx1 on test(v1); - ddl txn tries acquiring conflicting object lock on relation test leads to ddl_txn2 -> analyze_txn ``` The above leads to a deadlock. Since the session level txn has a inherent dependency onto the host txn, we end up with a false cycle ``` session_txn1 -> ddl_txn2 -> analyze_txn -> session_txn1 ``` This is due to the earlier edge `analyze_txn -> session_txn1` not being pruned. The wait-for edges in YB are pruned either when the corresponding subtxn rollsback or the txn itself isn't active anymore. This revision fixes the above false deadlock issue by associating the session object locks from different transactions to different subtxns. In specific, - when acquiring a session level object lock, if one doesn't already exist, bump up the active subtxn and acquire the session object locks against it (stored in `subtxn_with_session_object_locks_`). - when acquiring a session level object lock, if an active subtxn with session locks already exists, acquire them against the same subtxn - on release all session locks, rollback to `subtxn_with_session_object_locks_` and bump the active subtxn id The same session level txn is also used for acquiring session advisory locks, and they need to honored despite the above logic of rolling back specific subtxns. The above logic takes care of this since we increment active subtxn when acquiring a session object lock for the first time, ensuring that all active advisory locks associated with the current subtxn (before the increment) would still remain active. Additionally an `SCHECK` is introduced on the acquire session advisory lock path which ensures that we don't serve any session advisory lock requests when the session level txn is holding active session object locks. This is necessary as release all session object locks rollsback the subtxn. This SCHECK itself is expected to never fail since session advisory locks are user level and cannot be issue in the middle of execution of a DDL (like CREATE INDEX) which use session object locks. **Upgrade/Downgrade safety** Added new field to the proto message which is used for ysql <-> local tserver communication alone. No upgrade/downgrade impact. Additionally, the usage of the feature (table locking) is disabled by default in all existing major releases. Test Plan: Jenkins ./yb_build.sh --cxx-test pg_object_locks-test --gtest_filter PgObjectLocksTest.ConsecutiveCreateIndexDontDeadlock Reviewers: amitanand Reviewed By: amitanand Subscribers: yql, ybase Differential Revision: https://phorge.dev.yugabyte.com/D53287
| Commit: | c74b3d1 | |
|---|---|---|
| Author: | Sanketh I | |
| Committer: | Sanketh I | |
[#29647] YSQL: Avoid master query on PG startup for colocation info Summary: Every PG backend startup queries the master to learn whether its target database is colocated (added a while ago in f0082094c4583989665b72e78180d5874600090e). This diff introduces a tserver-side cache by db oid for this information to avoid this RPC. DB colocation information does not change once the db is created. 1. When multiple backends look up this information at the same time, only one outstanding query is made to the master and the remaining backends use the result of that query (both in success & failure cases). 2. If a query to the master for this info fails, there is no negative cache of the failed result. 3. When a db is dropped, heartbeats to the tserver inform of this event - this is currently used to keep the db oid -> catalog version map accurate and remove entries from it. The same path is used to also keep this cache up to date for dropped dbs. **Upgrade/Rollback safety:** # A new PG -> local tserver is introduced. PG and tserver are upgraded at the same time, so this should be safe. Test Plan: No new test is introduced. A manual test confirmed the behavior for regular hits/misses/drop db. Jenkins tests confirm that no breakage happened. Reviewers: myang, zdrudi Reviewed By: zdrudi Subscribers: yql, ybase, svc_phabricator Differential Revision: https://phorge.dev.yugabyte.com/D53350
| Commit: | 48fc88e | |
|---|---|---|
| Author: | Minghui Yang | |
| Committer: | Minghui Yang | |
[#28482] YSQL: implement new-relation fastpath write optimization (skip intents) Summary: This diff introduces the *new-relation fastpath write* optimization (informally "skip intents"). In a standard YB distributed transaction, all writes first go through the intents DB and are only moved to the regular DB on commit. This diff allows writes to a relation that was newly created (or had its physical storage swapped) in the same transaction to bypass the intents DB and write directly to the regular DB, and lets reads on those relations skip the intents merge as well. The result is significantly less I/O for CTAS, table rewrites, MV refresh, and bulk loads in DDL-only transactions. **Core decision logic — `YbCanSkipIntents(Relation rel, bool is_write)` in `pg_yb_utils.c`** Returns true (i.e., the optimization is safe) when all of the following hold: 1. The relation was created in the current transaction (`rd_createSubid != InvalidSubTransactionId`) or had its storage swapped in the current transaction (`rd_newRelfilenodeSubid != InvalidSubTransactionId`). The second case extends the optimization to `ALTER TABLE` rewrites, `REINDEX`, and `REFRESH MATERIALIZED VIEW` (non-concurrent). 2. The relation is not a system catalog (`rd_id >= FirstNormalObjectId`), not temporary, and not colocated. 3. The optimization has not been disabled earlier in this transaction. 4. The transaction has no non-read-committed *named* savepoint above the current sub-txn (`ROLLBACK TO SAVEPOINT` requires real intents). 5. The statement is top-level *unless* the in-txn-blocks preview GUC is on and the isolation level is READ COMMITTED — in which case non-top-level statements are allowed only when DDL transaction blocks (`ysql_yb_ddl_transaction_block_enabled`) are also enabled. For reads, an additional Halloween-problem guard runs in `YbMaybeDisableSkipIntentsForCurrentTxn()`: if a same-txn-created relation is read inside a function/trigger, from SPI, from a non-SELECT op, or from a SELECT with a modifying CTE, we permanently disable the optimization for the rest of the transaction so the next write does not corrupt isolation (e.g., `INSERT INTO t SELECT id+100 FROM t`). **GUCs** Two PGC_SUSET booleans (both refuse to change inside a transaction block, and both make the connection sticky in YSQL Conn-Mgr when set off-default): - `yb_enable_new_relation_fastpath_write` (default `true`): master switch. Top-level statements only. - `yb_enable_new_relation_fastpath_write_in_txn_blocks` (default `false`, preview-flag gated): allows fastpath inside `BEGIN`/transaction blocks. Only effective under READ COMMITTED + `ysql_yb_ddl_transaction_block_enabled`. **xCluster / CDC interaction** xCluster is fully compatible with the skip-intents optimization, as its table-level streams correctly replicate the direct fastpath writes. However, logical replication and legacy CDCSDK require intents, so we must not use fastpath when the database participates in CDCSDK. YbMaybeDisableSkipIntentsForCDCSDK() is called from the table-creation path and: 1. Checks whether `pg_publication` has any rows (publication-based CDCSDK). The result is session-cached and invalidated through a `PUBLICATIONOID` syscache callback so the scan only happens once per publication change. 2. If `ysql_cdcsdk_enable_old_namespace_streams` is true, falls back to a master RPC for old-style namespace-level streams (no slot). Failures are conservatively treated as "namespace is part of CDCSDK". The fallback is plumbed through a new `IsNamespacePartOfCDCSDK` RPC on both the master replication service and the PG client service: - New master RPC `master.IsNamespacePartOfCDCSDK(namespace_id)` implemented in `xrepl_catalog_manager.cc` by scanning `cdc_stream_map_`. - New tserver RPC `pg_client.IsNamespacePartOfCDCSDK(database_oid)` implemented in `pg_client_service.cc`, which translates oid → namespace id and forwards to master. - New ASH wait state `kIsNamespacePartOfCDCSDK` and `YBClient::IsNamespacePartOfCDCSDK` helper. **Wire protocol changes** - `pgsql_protocol.proto`: adds optional `bool skip_intents_write` to `PgsqlWriteRequestPB` and optional `bool skip_intents_read` to `PgsqlReadRequestPB`. - `pg_client.proto`: adds the new `IsNamespacePartOfCDCSDK` RPC + request/response messages. - `master_replication.proto`: adds the new `IsNamespacePartOfCDCSDK` RPC + request/response messages. All wire changes are additive optional fields / new RPCs and only flow on PG <-> tserver and tserver <-> master paths, so they are upgrade-safe. **YBClient batcher / async RPC** `Batcher` records whether all ops in a batch carry the skip-intents flag (writes and reads both). When set: - `Batcher::transaction()` returns `nullptr` so the op bypasses transaction metadata propagation and goes straight to the tablet as a non-transactional write/read. - `force_consistent_read` is set unconditionally, and any explicit `read_time` is cleared in `AsyncRpc::ProcessResponseFromTserver` to avoid "Restart read required" errors. - A new tablet-server counter `skip_intents_writes` is incremented per fastpath write batch. **PG executor / planner integration** - All `YbNewInsert/Update/Delete/InsertBlock` callers now pass `YbCanSkipIntentsWrite(rel)`; `YbNewSelect/Sample` pass `YbCanSkipIntentsRead(rel)`. The flag is plumbed through the entire pggate stack (`PgInsert/Update/Delete/Select/SelectIndex/Sample/SamplePicker`). - `pg_session.cc` flushes the buffered op queue when the skip-intents mode of the next op differs from the previous one (since fastpath and normal ops can't share a batch). - `nodeLockRows.c`: when fastpath is in effect we skip `YBCLockTuple` entirely — there's nothing in intents to lock. - `postgres.c::yb_is_retry_possible`: query-layer retry is disabled once any fastpath write has been issued in this transaction (data is already in regular DB; replaying would duplicate). - `xact.c`: `YbEnableSkipIntentsForNewTransaction()` resets per-txn state at every transaction start. - `Assert(!YbCanSkipIntentsWrite(rel))` was added to the catalog DELETE path in `ybModifyTable.c` as a safety net — catalogs must never use fastpath. - `YbGetSPIStackDepth()` (new in `spi.c`) is used to detect "we're inside a function/procedure". **Savepoint helper split** `YBTransactionContainsNonReadCommittedSavepoint()` (used by the skip-intents check) is now a separate helper that returns true for any non-RC subtransaction (named or anonymous). The original named-savepoint semantics are preserved under a new name, `YBTransactionContainsNonReadCommittedNamedSavepoint()`, which is what the existing "interleaving SAVEPOINT & DDL" check now uses. **API rename** `YbGetTableDistribution(Oid)` → `YbGetTableDistributionById(Oid)`; a new `YbGetTableDistribution(Relation)` overload is the preferred form when the caller already holds a `Relation` (used by the fastpath check itself). Callers in `costsize.c`, `allpaths.c`, and `pg_hint_plan/core.c` updated. **Upgrade / rollback safety** The src/yb/tserver/pg_client.proto and src/yb/common/pgsql_protocol.proto changes are only used in PG -> tserver communication which is upgrade safe. All new wire fields are additive optional fields, and the new RPCs follow the standard add-and-rolling-upgrade pattern: during a rolling upgrade, `IsNamespacePartOfCDCSDK` may not yet exist on the master — `YbMaybeDisableSkipIntentsForCDCSDK()` treats the RPC failure conservatively and disables the optimization for that transaction. The optimization is also gated by GUCs that default to off (in-txn-blocks variant) or to a behavior equivalent to today (top-level only path is on by default but produces identical visible behavior). Test Plan: - New `pg_skip_intents_metrics-test.cc` exercises the metric across CTAS, multiple ALTER TABLE rewrite shapes (type change, ADD/DROP PRIMARY KEY, volatile defaults, multi-index tables), `MATERIALIZED VIEW` refresh (concurrent vs non-concurrent), publication on/off, isolation-level matrix (RC / RR / SERIALIZABLE), the same-txn-created-read Halloween guards, and PITR over fastpath-written data. - New `xcluster/xcluster_ysql_skip_intents-test.cc` validates xCluster + automatic DDL replication when both clusters have skip-intents enabled (CTAS, ALTER rewrite, chained CTAS). - `pg_ddl_transaction-test::PgDdlTransactionTest.TestNoSkipIntentsWriteOnSavepoint` covers the named-savepoint disable path end-to-end. - `TestDdlSavepoints` and `TestDdlTransactionBlocks` randomly toggle the in-txn-block GUC to broaden coverage. `TestDdlTransactionBlocks` also runs the `yb_ddl_txn_block_schedule` regress schedule, which includes the scaled-up `yb.orig.ddl_txn_visibility` regress test that exercises the Halloween-problem patterns. Test commands: ``` ./yb_build.sh release --cxx-test pg_skip_intents_metrics-test ./yb_build.sh release --cxx-test xcluster_ysql_skip_intents-test ./yb_build.sh release --cxx-test pg_ddl_transaction-test --gtest_filter PgDdlTransactionTest.TestNoSkipIntentsWriteOnSavepoint ./yb_build.sh release --java-test 'org.yb.pgsql.TestDdlSavepoints' ./yb_build.sh release --java-test 'org.yb.pgsql.TestDdlTransactionBlocks' ``` TPC-C perf run: no regression observed with the GUC on or off. Reviewers: sanketh, pjain, sergei, timur, bkolagani, xCluster, patnaik.balivada, jhe Reviewed By: pjain, jhe Subscribers: smishra, dmitry, jason, hsunder, ybase, yql Differential Revision: https://phorge.dev.yugabyte.com/D48866
| Commit: | d76da5e | |
|---|---|---|
| Author: | Devansh Singhal | |
| Committer: | Devansh Singhal | |
[BACKPORT 2025.2][#29360] CDC: Stop relying on DELETING_METADATA state for stream metadata cleanup Summary: #### Backport Description Made some minor corrections in `cdcsdk_ysql-test.cc`. The `FLAGS_ysql_yb_enable_implicit_dynamic_tables_logical_replication` is **true by default on master** but is **false on 2025.2**. So, I set it to true here in tests. Also, it was needed to include "yb/master/catalog_manager.h" for tests since cdcsdk_ysql-test.cc was missing it in 2025.2. #### Original Description ##### Code changes summary Currently, once a table which was under CDC replication is dropped, the associated streams change their state to `DELETING_METADATA`. Once a stream goes in `DELETING_METADATA` state, it doesn't go back to `ACTIVE` state (a stream can now only transition to `DELETING` during its deletion). The Catalog Manager's background task `CleanUpCDCSDKStreamsMetadata()` takes the job of cleaning up of associated dropped tables' entries from cdc_state table and removing tables from stream metadata of such streams. For this, it iterates over the cdc_state table. Now since the stream remains in same state and this background task runs every `FLAGS_catalog_manager_bg_task_wait_ms` (default to **1 second**), it essentially iterates over cdc_state table each second (even though it may not have to do any additional task in next iterations if same streams are in DELETING_METADATA state). To mitigate this issue, we are altogether moving away from `DELETING_METADATA` state for relying on streams' metadata cleanup. With this revision, we are introducing a new table_id list in `SysCDCStreamEntryPB` which is `dropped_table_id`. The workflow pertaining to use of this list is as: * When a table used to get dropped, the associated streams were marked as `DELETING_METADATA`. Now, we will add such table to this newly introduced list for each associated stream. * The `CleanUpCDCSDKStreamsMetadata()` task will now query if any stream in `cdc_stream_map_` has non-empty `dropped_table_id` list. If so, then that stream is eligible for cleanup. Further in this function, `GetValidTabletsAndDroppedTablesForStream()` functions used to give all tablets to keep per stream and all tables dropped per stream. Now, this function only provides tablets to keep per stream (since we already have the dropped tables list from stream metadata). This way once the stream metadata is cleaned up, the `dropped_table_id` list will become empty for the stream thus preventing the repeatable execution of workflow of `CleanUpCDCSDKStreamsMetadata()` (thus also helping in reducing iterations of cdc_state table significantly). It is to be **note** here that this revision doesn't add `dropped_table_id` list in response of `GetCDCDBStreamInfo()` master RPC. This is intently left as such because the same RPC is used by `yb-admin`'s `get_change_data_stream_info` cmd and it prints all response proto (without filtering anything from it). Since `dropped_table_id` is an internal state for a stream, we do not intend to present it to user. Thus, made no change to response of `GetCDCDBStreamInfo()` RPC. It is noticed that there's an existing bug where when a table is dropped, the function `CatalogManager::DropCDCSDKStreams` (which used to mark associated streams as `DELETING_METADATA`) removes entry of table from `cdcsdk_tables_to_stream_map_`. If the background task `CatalogManager::CleanUpCDCSDKStreamsMetadata` doesn't run and master restarts, then during loading of streams from persisted data, the entry for such table is again added to `cdcsdk_tables_to_stream_map_`. To mitigate this: Now while loading the streams from persisted data, the table is first checked that it is available and not deleted. It this criteria is satisfied then only its entry is added to `cdcsdk_tables_to_stream_map_`. ##### Upgrade/rollback scenarios considerations **Scenario 1:** There may be streams from old version which may be present in `DELETING_METADATA` state. Some might even be those for which background task `CleanUpCDCSDKStreamsMetadata()` never ran once before cluster upgraded. Now with the new mechanism of `CleanUpCDCSDKStreamsMetadata()`, such streams metadata won't get cleaned up since `CleanUpCDCSDKStreamsMetadata()` no more uses `DELETING_METADATA` state and such old streams don't have `dropped_table_id` list populated in their metadata. To mitigate this situation: When the `CleanUpCDCSDKStreamsMetadata()` function runs, we find the streams which are in `DELETING_METADATA` state. We then find out all the tables which can be dropped and thus add them in `dropped_table_id` list. We also mark such stream's state to `ACTIVE`. Thus, now their cleanup can proceed as per new logic of `CleanUpCDCSDKStreamsMetadata()`. **Scenario 2:** There's also a scenario where a universe with old binary when upgraded to newer one containing this change, the logic populates the `dropped_table_ids` for streams with `DELETING_METADATA` when being loaded from persisted data and mark them as 'ACTIVE`. However if at this time the upgrade is rolled back, then streams which had `DELETING_METADATA` state is now changed to `ACTIVE` and so their cleanup will never happen. To mitigate this case, the change is wrapped in a preview flag `cdcsdk_use_dropped_table_list_for_cleanup` which eventually in next revisions will be promoted to `auto` flag with default `true` value. ##### Upgrade/Rollback safety Revision adds a new field `dropped_table_id` in message `SysCDCStreamEntryPB`. This is a repeated field and doesn't cause any issue with upgrade and rollback. The workflows using this new field handle the management of this field accordingly. As mentioned earlier, the change is wrapped in a preview flag `cdcsdk_use_dropped_table_list_for_cleanup` which eventually in next revisions will be promoted to `auto` flag with default `true` value. ##### Considerations for colocated tables Colocated tables are being handled the same way as before the revision. ##### Compatibility with logical and gRPC streams The revision handles both type of streams. ##### Considerations for connector NA Jira: DB-19153 Original commit: 86d06e610f733d59be2cd49cdc766baafb338a7e / D50213 Test Plan: yb_build.sh --cxx-test cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestPopulationOfDroppedTableListInStreamMetadata yb_build.sh --cxx-test cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestUpgradeFromDeletingMetadataToDroppedTableList yb_build.sh --cxx-test cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestDropStreamDuringUpgradeFromDeletingMetadataToDroppedTableList yb_build.sh --cxx-test cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestDropTableDuringUpgradeFromDeletingMetadataToDroppedTableList Reviewers: sumukh.phalgaonkar, asrinivasan, skumar, xCluster, hsunder, #db-approvers, ssharma Reviewed By: #db-approvers, ssharma Subscribers: svc_phabricator, ycdcxcluster, ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D53007
| Commit: | a305dda | |
|---|---|---|
| Author: | jhe | |
| Committer: | jhe | |
[#31402] docdb: Use TabletInvoker for async writes Summary: Make pipelined async writes survive a leader stepdown when the writes were committed before the stepdown. - `WaitForAsyncWriteRpc` now uses `TabletInvoker`. On `NOT_THE_LEADER` errors from the original leader, it will retry on the new leader - `TabletPeer::VerifyAsyncWriteCompletion` runs on the new leader when the OpId isn't in its in-flight tracker (ie the write went to the previous leader). It checks `op_id.index < first_index_of_current_term_` to decide whether the previous-term entry was committed - If yes, then we can proceed - if no, then we return Aborted and abort the transaction. - For now `VerifyAsyncWriteCompletion` only handles writes from the current and previous term; if the term difference is > 2, then we also abort the transaction. If this is needed in the future, we can do a log lookup to verify the term and index. Also removing usage of `leader_term` for async writes, since this was only used to abort transactions on leadership changes before. **Upgrade/Rollback safety:** The feature is gated by gFlag `ysql_enable_write_pipelining`. The new `ReadRequestPB.pending_async_write_op_id` field is optional and defaults to unset on older servers, which fall back to the previous (no verification) behavior. Test Plan: async_writes-test Reviewers: hsunder, sergei Reviewed By: hsunder Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D51088
| Commit: | 0426fb9 | |
|---|---|---|
| Author: | Eric Sheng | |
| Committer: | Eric Sheng | |
[#26950] docdb: Add support for proto3 optional fields Summary: Protobuf 3.12 added support for optional fields when using proto3. However, our code generators for lightweight protobufs do not support it properly, since proto3 optional fields are treated like: ``` oneof _foo { int32_t foo = 1; [proto3_optional=true] } ``` instead of like proto2 optional fields. Additionally, even the protoc-gen-insertions generator which just adds an include at the top does not work since it does not signal that it supports proto3 optional. The changes needed to support proto3 optional fields, which this change implements, are: - Add `GetSupportedFeatures()` on `CodeGenerator`s returning `FEATURE_PROTO3_OPTIONAL` to indicate that our generators support proto3 optionals. - Change `containing_oneof()` to `real_containing_oneof()` to skip over the synthetic `oneof`s generated by proto3 `optional`. Our lightweight protobuf generator already ignores the proto2/proto3 distinction and generates all normal fields like proto2 `optional` by default, so by skipping the synthetic `oneof`s, proto3 `optional` fields are now correctly treated like proto2 `optional` fields for generation. **Upgrade/Rollback Safety** N/A, only modifies pg_client.proto used for tserver/pg communication. Test Plan: Jenkins. Also checked that the generated code for `colocation_id` in `pg_client.messages.*` matches other fields. Reviewers: sergei Reviewed By: sergei Subscribers: ybase, yql Differential Revision: https://phorge.dev.yugabyte.com/D43514
| Commit: | 6134453 | |
|---|---|---|
| Author: | jhe | |
| Committer: | jhe | |
[BACKPORT 2025.2.3][#31533] xCluster: Fix SIGSEGV on upgrade due to empty old_consumer_schema_versions Summary: Adding a `RepairOldSchemaVersionsPB` helper that checks for missing `old_*_schema_version` fields and backfills the empty side with a 0. This helper is run on syscatalog load and fixes the in-memory state so that we maintain the equal sized `old_*_schema_versions` lists. This doesn't persist anything to syscatalog, that will be done on the next schema version bump as `UpdateConsumerOnProducerMetadata` will fully recreate and persist these fields. Adding `[packed = false]` on `SchemaVersionsPB.old_*_schema_versions`. This restores wire-type-0 (per-element tag+varint) so a pre-D45888 reader can parse a 1-element list as the singular value. **Upgrade/Rollback safety:** `[packed = false]` only changes what the writer emits, the readers are able to accept either encoding Original commit: ce7a148c4afa1bc6b0eddc144927165b9f2c9cc6 / D52960 Test Plan: ``` ybd --cxx-test xcluster_ddl_replication-test --gtest_filter 'XClusterDDLReplicationTest.RepairOldSchemaVersionsOnLoad' ``` Reviewers: hsunder, xCluster Reviewed By: hsunder Subscribers: xCluster, ybase Differential Revision: https://phorge.dev.yugabyte.com/D53325
| Commit: | 36519c9 | |
|---|---|---|
| Author: | Naorem Khogendro Singh | |
| Committer: | Naorem Khogendro Singh | |
[PLAT-20859] SSH Key Rotation is Broken After Ansible Removal Summary: Ansible task was removed. This places it with node agent implementation. Breakdown: 1. RotateSshKey RPC is added to add a new key and remove an old key. It can function as only remove or only add or both add and remove in a single call. 2. Add and Remove handlers in python layer are removed (replaced with node agent implementation). 3. RotateAccessKey Java task does not need to check the initial SSH connection test. Test Plan: UTs added for golang changes. Manually tested by rotating it multile times using the REST APIs. 1. Create an access key //jenk-id19813-d160c7cb2f-20260512-162724-key// using curl -k -X POST 'http://localhost:9000/api/v1/customers/f33e3c9b-75ab-4c30-80ad-cba85646ea39/providers/ed205728-046b-46f7-b6f9-c22bdd334296/access_keys .... 2. Rotate to that key in 1 using curl -k -X POST 'http://localhost:9000/api/v1/customers/f33e3c9b-75ab-4c30-80ad-cba85646ea39/providers/ed205728-046b-46f7-b6f9-c22bdd334296/access_key_rotation' .... After the rotation. ``` (venv) ns-mbp-yv909:devops nkhogen$ ssh -i /opt/yugaware/keys/ed205728-046b-46f7-b6f9-c22bdd334296/yb-admin-aws-new_ed205728-046b-46f7-b6f9-c22bdd334296-key.pem -ostricthostkeychecking=no -p 22 yugabyte@10.9.78.62 yugabyte@10.9.78.62: Permission denied (publickey,gssapi-keyex,gssapi-with-mic). (venv) ns-mbp-yv909:devops nkhogen$ ssh -i /opt/yugaware/keys/ed205728-046b-46f7-b6f9-c22bdd334296/jenk-id19813-d160c7cb2f-20260512-162724-key.pem -ostricthostkeychecking=no -p 22 yugabyte@10.9.78.62 Last login: Wed May 13 22:15:03 2026 from 100.107.149.122 [yugabyte@ip-10-9-78-62 ~]$ logout Connection to 10.9.78.62 closed. (venv) ns-mbp-yv909:devops nkhogen$ ssh -i /opt/yugaware/keys/ed205728-046b-46f7-b6f9-c22bdd334296/jenk-id19813-d160c7cb2f-20260512-162724-key.pem -ostricthostkeychecking=no -p 22 ec2-user@10.9.78.62 Last login: Wed May 13 22:00:25 2026 from 100.107.149.122 [ec2-user@ip-10-9-78-62 ~]$ logout ``` Old key is invalid. Reviewers: amindrov, spothuraju, skhilar, anijhawan, nbhatia Reviewed By: skhilar Subscribers: nikhil, yugaware Differential Revision: https://phorge.dev.yugabyte.com/D53143
| Commit: | 8e76d95 | |
|---|---|---|
| Author: | jhe | |
| Committer: | jhe | |
[BACKPORT 2025.2][#31533] xCluster: Fix SIGSEGV on upgrade due to empty old_consumer_schema_versions Summary: Adding a `RepairOldSchemaVersionsPB` helper that checks for missing `old_*_schema_version` fields and backfills the empty side with a 0. This helper is run on syscatalog load and fixes the in-memory state so that we maintain the equal sized `old_*_schema_versions` lists. This doesn't persist anything to syscatalog, that will be done on the next schema version bump as `UpdateConsumerOnProducerMetadata` will fully recreate and persist these fields. Adding `[packed = false]` on `SchemaVersionsPB.old_*_schema_versions`. This restores wire-type-0 (per-element tag+varint) so a pre-D45888 reader can parse a 1-element list as the singular value. **Upgrade/Rollback safety:** `[packed = false]` only changes what the writer emits, the readers are able to accept either encoding Original commit: ce7a148c4afa1bc6b0eddc144927165b9f2c9cc6 / D52960 Test Plan: ``` ybd --cxx-test xcluster_ddl_replication-test --gtest_filter 'XClusterDDLReplicationTest.RepairOldSchemaVersionsOnLoad' ``` Reviewers: hsunder, xCluster Reviewed By: hsunder Subscribers: ybase, xCluster Differential Revision: https://phorge.dev.yugabyte.com/D53156
| Commit: | c03755b | |
|---|---|---|
| Author: | Basava | |
| Committer: | Basava | |
[BACKPORT 2025.1][#28463] DocDB: Fix TServer crash on shutdown when concurrent raft op apply asserts TSTabletManager to be running Summary: Operations like `consensus::SPLIT_OP` and `consensus::CLONE_OP` require `TSTabletManager` to be in `MANAGER_RUNNING` state during raft apply `Operation::Replicated`. In the current code, we seem to set the shutdown state for `TSTabletManager` prior to setting shutdown state on the tablet peers. This leads to tserver crashes closer to shutdown if a split/clone op is being applied with the following log ``` F20250901 18:59:07 ../../src/yb/tablet/operations/operation_driver.cc:422] T 087c2e8412254eab823ea4dfdd3fa091 P {ts2_peer_id} S RD-P Ts { days: 20332 time: 18:59:07.479471 } kSplit (0x000072280015eaa0): Apply failed: Illegal state (yb/tserver/ts_tablet_manager.cc:1151): Manager is not running: 2, request: dest_uuid: "{ts2_peer_id}" propagated_hybrid_time: 7195660892071751680 tablet_id: "087c2e8412254eab823ea4dfdd3fa091" new_tablet1_id: "681171c81c424a1e9abd641bd10027bf" new_tablet2_id: "9b26091fb731444292ff4ed78adf15fd" split_partition_key: "44C8" split_encoded_key: "4744C8" split_parent_leader_uuid: "{ts2_peer_id}" ``` This revision addresses the above issue by introducing another state `TSTabletManagerStatePB ::MANAGER_STARTED_QUIESCING` which is considered as `TSTabletManager::IsOperational()` and allows apply of ops. `TSTabletManager::state_` is set to `MANAGER_STARTED_QUIESCING` at the start of `TSTabletManager::StartShutdown` and is transitioned to `MANAGER_QUIESCING` at the end. Additionally, we pre-pone the state transition to `MANAGER_RUNNING` before `OpenTablet` tasks are submitted to the threadpool. Else, this too could result in similar fatals, as `OpenTablet` would start consensus and we might trigger apply of ops even before `TSTabletManager` switches to `MANAGER_RUNNING`. **Upgrade / Downgrade safety** Added a type to `enum TSTabletManagerStatePB`. the type itself isn't being sent over the wire, so there shouldn't be any upgrade/ downgrade issues. Original commit: e524989ef94a8a0d110f5c110e2691cdb0f6859c / D51518 Test Plan: Jenkins ./yb_build.sh --cxx-test integration-tests_tablet-split-itest --gtest_filter TabletSplitITest.SplitApplyAfterTabletManagerStartShutdownBegins -n 10 --tp 1 Test fails consistently without the changes. Reviewers: sergei, esheng, arybochkin Reviewed By: esheng Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D52987
| Commit: | 78af3a2 | |
|---|---|---|
| Author: | Timur Yusupov | |
| Committer: | Timur Yusupov | |
[#27056] docdb: Fixed tablet split vs RBS from the follower race Summary: There is a possibility of tablet split vs RBS from follower race: 1. Parent tablet leader peers A-C accept a SPLIT_OP (op_id: 1.4). Leader (A) and 1st follower (B) apply SPLIT_OP, 2nd follower (C) doesn't apply it yet. 2. Parent tablet leader (node A) accepts CHANGE_CONFIG_OP (op_id: 1.5) to add a fourth peer (D) but doesn't apply it yet. 3. Parent tablet 2nd follower (C) still hasn't yet applied SPLIT_OP (1.4). 4. RBS for parent tablet peer (D) starts from the follower (C) and tablet metadata (tablet_data_state == TABLET_DATA_READY) is downloaded. 5. Parent tablet peers A-C completed applying the SPLIT_OP (1.4), child tablets have Raft config with 3 peers. 6. Parent tablet peers A-C apply CHANGE_CONFIG_OP (1.5) and now have committed Raft config with 4 peers. 7. Parent tablet peer D does local bootstrap and replays SPLIT_OP (1.4) as part of bootstrap. Due to tablet_data_state is TABLET_DATA_READY but not TABLET_DATA_SPLIT_COMPLETED replay does SPLIT_OP apply and creates child tablet peer. After that, 4th child tablet peer (D) is not a part of Raft group (which has 3 peers) and therefore is not receiving consensus updates from leader. This change fixes this race by rejecting RBS from the follower that is in progress of applying SPLIT_OP and RBS attempt will be retried later. **Upgrade/Rollback safety:** New error code will be printed by old nodes as just number in case of RBS failure during upgrade but this is safe. Test Plan: TabletSplitITest.SplitWithParentTabletRbsFromFollower, TabletSplitITest.SplitWithParentTabletMove, RemoteBootstrapsFromNodeWithUncommittedSplitOp - 30 runs per each of asan/tsan/debug/release builds Reviewers: asrivastava Reviewed By: asrivastava Subscribers: zdrudi, ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D48853
| Commit: | 2f2fa0a | |
|---|---|---|
| Author: | Yamen Haddad | |
| Committer: | Yamen Haddad | |
[BACKPORT 2026.1][#31482] Docdb: Handle quoted identifiers in clone Summary: Original commit: 95306ec58b4b56c50b822f7b4d6576ef96af3b12 / D52856 This change fixes owner rewriting in clone pg schema path. The diff removes the regex post processing in favor of a more robust approach where the rewrite happens withing the ysql_dump tool itself. To do that 2 new ysql_dump options where added as follows: 1- `--rename-database=new_db_name`: overrides the local datname once in `dumpDatabase()`, which transparently propagates to every `CREATE/ALTER/COMMENT/SECURITY LABEL/GRANT/REVOKE on DATABASE` and to the `\connect` line. 2- `--rename-owner=new_db_owner`: captures the source DB owner from `pg_database.datdba` inside `dumpDatabase()` and routes every OWNER TO emission through a single helper (`yb_effective_owner` in `pg_backup_archiver.c`) that substitutes the target owner only when the entry's owner equals the source DB owner. The clone path no longer needs to pass the source DB role name across the RPC chain — ysql_dump derives it itself — and the previous regex-based owner/DB-name rewriting in YsqlDumpRunner is gone. **Upgrade/Rollback safety:** Removed the `source_owner` field from the 3 RPCs used in the clone chain. The removal is wire-compatible (protobuf treats absent fields as default), but a new master sending an empty value to an old tserver corrupts owner clauses in its legacy regex-based dump rewriter and aborts the clone with a `clear role "..." does not exist error`. This only affects users during a rolling upgrade window and only for users who had explicitly enabled the EA flag `--enable_db_clone=true` on the prior release; fresh 2026.1 universes and pre-2026.1 users who haven't enabled clone in TP or EA are protected automatically because the autoflag `enable_db_clone` (kLocalPersisted) stays unpromoted until every node is on the new binary. Prioritized simplicity over dealing with this very edge case, as the mitigation is to retry cloning after fully upgrading to the new version. Test Plan: ./yb_build.sh --cxx-test integration-tests_minicluster-snapshot-test --gtest_filter PgCloneTest.CloneWithQuotedSourceOwner ./yb_build.sh --cxx-test integration-tests_minicluster-snapshot-test --gtest_filter PgCloneTest.CloneWithSpecialCharsInOwner ./yb_build.sh --cxx-test integration-tests_minicluster-snapshot-test --gtest_filter PgCloneTest.CloneRewritesOwnersOfSourceOwnedObjects ./yb_build.sh --cxx-test integration-tests_minicluster-snapshot-test --gtest_filter PgCloneTest.CloneWithBackslashInDbName Also added Java tests to test ysql_dump output: ./yb_build.sh --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpRenameDatabase' ./yb_build.sh --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpRenameOwner' ./yb_build.sh debug --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpRenameRequiresCreate' Reviewers: zdrudi, loginov Reviewed By: zdrudi Subscribers: ybase, hsunder, neera.mital, yql Differential Revision: https://phorge.dev.yugabyte.com/D53178
| Commit: | 3a5b5c4 | |
|---|---|---|
| Author: | jhe | |
| Committer: | jhe | |
[BACKPORT 2026.1][#31533] xCluster: Fix SIGSEGV on upgrade due to empty old_consumer_schema_versions Summary: Adding a `RepairOldSchemaVersionsPB` helper that checks for missing `old_*_schema_version` fields and backfills the empty side with a 0. This helper is run on syscatalog load and fixes the in-memory state so that we maintain the equal sized `old_*_schema_versions` lists. This doesn't persist anything to syscatalog, that will be done on the next schema version bump as `UpdateConsumerOnProducerMetadata` will fully recreate and persist these fields. Adding `[packed = false]` on `SchemaVersionsPB.old_*_schema_versions`. This restores wire-type-0 (per-element tag+varint) so a pre-D45888 reader can parse a 1-element list as the singular value. **Upgrade/Rollback safety:** `[packed = false]` only changes what the writer emits, the readers are able to accept either encoding Original commit: ce7a148c4afa1bc6b0eddc144927165b9f2c9cc6 / D52960 Test Plan: ``` ybd --cxx-test xcluster_ddl_replication-test --gtest_filter 'XClusterDDLReplicationTest.RepairOldSchemaVersionsOnLoad' ``` Reviewers: hsunder, xCluster Reviewed By: hsunder Subscribers: xCluster, ybase Differential Revision: https://phorge.dev.yugabyte.com/D53155
| Commit: | b2a69b2 | |
|---|---|---|
| Author: | Devansh Singhal | |
| Committer: | Devansh Singhal | |
[BACKPORT 2024.2.9][#23497] CDC: Clean stale entries from cdc_state table Summary: ##### Backport Description Resolved few minor merge conflicts in cdc_service.cc, xrepl_catalog_manager.cc and cdcsdk_consistent_snapshot.cc files. Those were just related to code placement. In test file cdcsdk_consistent_snapshot.cc, conflicts came due to incoming changes which had FLAGS_ysql_yb_enable_implicit_dynamic_tables_logical_replication and FLAGS_ysql_ddl_rpc_timeout_sec in it. Removed these lines since such flags are not defined in 2024.2 (i.e their revisions are not present in 2024.2). Had a compilation error that `last_seen_tablet_stream_entries_` required a `std::hash` specialization for `TabletStreamInfo`, but none existed since `last_seen_tablet_stream_entries_` was being defined as `std::shared_ptr<std::unordered_set<TabletStreamInfo>` without specifying the hash function. As a fix, `TabletStreamInfo` already has a hash function defined so the same is provided in `last_seen_tablet_stream_entries_`'s declaration. An already existing local variable `expired_entries` in `CDCServiceImpl::UpdateMetrics()` uses the same in its declaration. This issue didn't occurred on master because on master `TabletStreamInfo` already uses the `YB_STRUCT_DEFINE_HASH` macro which automatically provide `std::hash<TabletStreamInfo>`. Additionally made default value of flag `cdc_min_replicated_index_considered_stale_secs` to 60 mins (instead of 30 mins as it is in original commit). ##### Original Description It is seen that when a database with a replication slot is dropped followed by the stream drop, the current logic fails to delete the related tablet-stream entries as well as slot entry from the cdc_state table. This issue occurs because: - when a **DB drop** is issued, the associated tables mark the related streams with `DELETING_METADATA` state. - There's a background task which responds on `DELETING_METADATA` state and removes associated tablet-stream entries from cdc state table. - However, it won't get a chance to run if **Stream drop** is issued before its run. This is because **stream delete** workflow overwrites stream state to `DELETING`. - The other background task which responds on `DELETING` state updates the checkpoint of associated tablet stream entries and slot entry to max. - UpdatePeersAndMetrics() (aka UPAM) tries to delete the entries which have max checkpoint. But it first tries to update all the associated tablet peers about the max checkpoint (so that each tablet peers can remove the retention barriers and so release their resources (such as WAL logs, Intent's SST files, History SST files)). It fails here since the tablet peers are already gone (as part of table deletion). - This way such entries always linger in cdc state table. This diff solves this issue in 2 sub-solutions: 1. It restricts moving stream to any other state if stream is already in `DELETING` (this is so because for example overwriting state DELETING to DELETING_METADATA may cause stale slot entry to remain in state table). 2. It lets all the workflows, which were previously setting the concerned cdc stable table entries checkpoint to OpId::Max() (so that UPAM get to know about such entries and can update peers about max checkpoint and then delete such entries), instead delete such entries rightaway. This is because diff removes the restrictions from UPAM to update peers about max checkpoint. The retention barriers on peers will eventually go stale and associated resources will be released using a newly introduced maintenance op i.e. `ResetStaleRetentionBarriersOp`. - This op checks if the barriers have gone stale (using `TabletPeer::is_cdc_min_replicated_index_stale()`) and if this op had ran before the refresh of `cdc_min_replicated_index`. If so, then it shows its interest of execution to `MaintenanceManager`. - Once it executes, it resets all cdc retention barriers using `TabletPeer::reset_all_cdc_retention_barriers_if_stale()`. **Note that** we are still keeping the logic of deleting entries with max checkpoint in UPAM so as to ensure that all stale entries gets deleted if some workflows still updating entries' checkpoint to OpId::Max(). - For removal of tablet metrics for a stream which are not required, we now take a set difference of tablet-stream entries present in cdc state table during previous iteration of `CDCServiceImpl::UpdateMetrics()` vs now. If some of the tablet-stream entries are missing in latest iteration, then metrics for such stream is removed from concerned tablet. for this, we use a newly introduced in-mem variable `last_seen_tablet_stream_entries_` instead of `CDCServiceImpl::Impl's tablet_checkpoints_` (This is necessary because `tablet_checkpoints_` insert tablet-stream elements only during a GetChanges call, so there might be some tablets which are never polled and so the previous logic fails to remove metrics for such tablets). The diff also changes the default value of gflag "cdc_min_replicated_index_considered_stale_secs" from 15 mins to 30 mins. Upgrade/Rollback safety: - Diff adds the repeated field `deleted_tablet_entries` and deprecates field `updated_tablet_entries` in `ValidateAndSyncCDCStateEntriesForCDCSDKStreamResponsePB` message in `master_replication.proto`: This is not a concern for upgrades and rollback since the above message is being used for `yb-admin`'s CDCSDK cmd `validate_and_sync_cdc_state_table_entries_on_change_data_stream`. A node (on older version) consuming the response from node (on newer version) will discard the newer field values making the output of cmd incomplete. However, internally, the data and its state remains consistent. - Diff also add optional field `cdcsdk_reset_stale_retention_barrier` in message `MaintenanceOpPB` in `tablet.proto`: This is also okay from upgrade and rollback perspective since the producer and consumer of this proto message lies on same node. Jira: DB-12410 Original commit: a71f00f9b317138c318823901201a9f8198d820c / D52432 Test Plan: Jenkins: urgent The diff runs all tests as part of test plan. yb_build.sh --cxx-test cdcsdk_consumption_consistent_changes-test --gtest_filter=CDCSDKConsumptionConsistentChangesTest.TestRetentionBarrierPreservedUntilStale yb_build.sh --cxx-test cdcsdk_consumption_consistent_changes-test --gtest_filter=CDCSDKConsumptionConsistentChangesTest.CheckSlotRowDeletionForStreamAndTableDeletion yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestMetricObjectRemovalAfterNamespaceDeletion yb_build.sh --cxx-test integration-tests_cdcsdk_consistent_snapshot-test --gtest_filter CDCSDKConsistentSnapshotTest.TestRetentionBarrierSettingRace yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestValidationAndSyncOfCDCStateEntriesAfterUserTableRemovalOnNonConsistentSnapshotStream yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestValidationAndSyncOfCDCStateEntriesAfterUserTableRemovalOnConsistentSnapshotStream yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestDeletedStreamRowsRemoved yb_build.sh --cxx-test maintenance_manager-test --gtest_filter=MaintenanceManagerTest.TestRegisterUnregister yb_build.sh --cxx-test maintenance_manager-test --gtest_filter=MaintenanceManagerTest.TestCompletedOpsHistory Reviewers: sumukh.phalgaonkar, skumar, stiwary, asrinivasan, xCluster, hsunder Reviewed By: sumukh.phalgaonkar Subscribers: schandra, ybase, svc_phabricator, ycdcxcluster Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D52735
| Commit: | d0a183c | |
|---|---|---|
| Author: | Anton Rybochkin | |
| Committer: | Anton Rybochkin | |
[BACKPORT 2025.2][#31350] docdb: Make admin compaction commands compact vector indexes by default Summary: Currently, the yb-ts-cli compact_tablet / compact_all_tablets commands do not consider vector indexes located on the given tablet and require an additional parameter, `--include_vector_indexes`, to compact vector indexes as well. This behavior is counterintuitive -- vector indexes should also be compacted when the corresponding compact_tablet commands are run. This change updates the behavior to include all vector indexes located on the tablet during compaction. However, since vector index compaction can be significantly heavier than RocksDB compactions of the same size, an additional parameter, `--exclude_vector_indexes`, is introduced to allow skipping vector indexes during the operation. **Upgrade/Rollback safety:** FlushCompactFlags is extended with a new item, which is simply ignored on the older version. Hence no impacts on upgrades/rollbacks. Original commit: f6d96a43a26b56d89b76208d440fd9069abf4916 / D52818 Test Plan: ./yb_build.sh --cxx-test yb-ts-cli-test --gtest_filter "YBTsCliTest.*" ./yb_build.sh --cxx-test tablet_server-test --gtest_filter "FlushAndCompact/*" Reviewers: sergei, hsunder, zdrudi Reviewed By: hsunder Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D53131
| Commit: | 2a5650a | |
|---|---|---|
| Author: | Anton Rybochkin | |
| Committer: | Anton Rybochkin | |
[BACKPORT 2026.1][#31350] docdb: Make admin compaction commands compact vector indexes by default Summary: Currently, the yb-ts-cli compact_tablet / compact_all_tablets commands do not consider vector indexes located on the given tablet and require an additional parameter, `--include_vector_indexes`, to compact vector indexes as well. This behavior is counterintuitive -- vector indexes should also be compacted when the corresponding compact_tablet commands are run. This change updates the behavior to include all vector indexes located on the tablet during compaction. However, since vector index compaction can be significantly heavier than RocksDB compactions of the same size, an additional parameter, `--exclude_vector_indexes`, is introduced to allow skipping vector indexes during the operation. **Upgrade/Rollback safety:** FlushCompactFlags is extended with a new item, which is simply ignored on the older version. Hence no impacts on upgrades/rollbacks. Original commit: f6d96a43a26b56d89b76208d440fd9069abf4916 / D52818 Test Plan: ./yb_build.sh --cxx-test yb-ts-cli-test --gtest_filter "YBTsCliTest.*" ./yb_build.sh --cxx-test tablet_server-test --gtest_filter "FlushAndCompact/*" Reviewers: sergei, hsunder, zdrudi Reviewed By: hsunder Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D53130
| Commit: | f589086 | |
|---|---|---|
| Author: | Basava | |
| Committer: | Basava Kolagani | |
[BACKPORT 2026.1][#28463] DocDB: Fix TServer crash on shutdown when concurrent raft op apply asserts TSTabletManager to be running Summary: Operations like `consensus::SPLIT_OP` and `consensus::CLONE_OP` require `TSTabletManager` to be in `MANAGER_RUNNING` state during raft apply `Operation::Replicated`. In the current code, we seem to set the shutdown state for `TSTabletManager` prior to setting shutdown state on the tablet peers. This leads to tserver crashes closer to shutdown if a split/clone op is being applied with the following log ``` F20250901 18:59:07 ../../src/yb/tablet/operations/operation_driver.cc:422] T 087c2e8412254eab823ea4dfdd3fa091 P {ts2_peer_id} S RD-P Ts { days: 20332 time: 18:59:07.479471 } kSplit (0x000072280015eaa0): Apply failed: Illegal state (yb/tserver/ts_tablet_manager.cc:1151): Manager is not running: 2, request: dest_uuid: "{ts2_peer_id}" propagated_hybrid_time: 7195660892071751680 tablet_id: "087c2e8412254eab823ea4dfdd3fa091" new_tablet1_id: "681171c81c424a1e9abd641bd10027bf" new_tablet2_id: "9b26091fb731444292ff4ed78adf15fd" split_partition_key: "44C8" split_encoded_key: "4744C8" split_parent_leader_uuid: "{ts2_peer_id}" ``` This revision addresses the above issue by introducing another state `TSTabletManagerStatePB ::MANAGER_STARTED_QUIESCING` which is considered as `TSTabletManager::IsOperational()` and allows apply of ops. `TSTabletManager::state_` is set to `MANAGER_STARTED_QUIESCING` at the start of `TSTabletManager::StartShutdown` and is transitioned to `MANAGER_QUIESCING` at the end. Additionally, we pre-pone the state transition to `MANAGER_RUNNING` before `OpenTablet` tasks are submitted to the threadpool. Else, this too could result in similar fatals, as `OpenTablet` would start consensus and we might trigger apply of ops even before `TSTabletManager` switches to `MANAGER_RUNNING`. **Upgrade / Downgrade safety** Added a type to `enum TSTabletManagerStatePB`. the type itself isn't being sent over the wire, so there shouldn't be any upgrade/ downgrade issues. Original commit: e524989ef94a8a0d110f5c110e2691cdb0f6859c / D51518 Test Plan: Jenkins ./yb_build.sh --cxx-test integration-tests_tablet-split-itest --gtest_filter TabletSplitITest.SplitApplyAfterTabletManagerStartShutdownBegins -n 10 --tp 1 Test fails consistently without the changes. Reviewers: sergei, esheng, arybochkin Reviewed By: esheng Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D52985
| Commit: | 6d7aee6 | |
|---|---|---|
| Author: | Basava | |
| Committer: | Basava Kolagani | |
[BACKPORT 2025.2][#28463] DocDB: Fix TServer crash on shutdown when concurrent raft op apply asserts TSTabletManager to be running Summary: Operations like `consensus::SPLIT_OP` and `consensus::CLONE_OP` require `TSTabletManager` to be in `MANAGER_RUNNING` state during raft apply `Operation::Replicated`. In the current code, we seem to set the shutdown state for `TSTabletManager` prior to setting shutdown state on the tablet peers. This leads to tserver crashes closer to shutdown if a split/clone op is being applied with the following log ``` F20250901 18:59:07 ../../src/yb/tablet/operations/operation_driver.cc:422] T 087c2e8412254eab823ea4dfdd3fa091 P {ts2_peer_id} S RD-P Ts { days: 20332 time: 18:59:07.479471 } kSplit (0x000072280015eaa0): Apply failed: Illegal state (yb/tserver/ts_tablet_manager.cc:1151): Manager is not running: 2, request: dest_uuid: "{ts2_peer_id}" propagated_hybrid_time: 7195660892071751680 tablet_id: "087c2e8412254eab823ea4dfdd3fa091" new_tablet1_id: "681171c81c424a1e9abd641bd10027bf" new_tablet2_id: "9b26091fb731444292ff4ed78adf15fd" split_partition_key: "44C8" split_encoded_key: "4744C8" split_parent_leader_uuid: "{ts2_peer_id}" ``` This revision addresses the above issue by introducing another state `TSTabletManagerStatePB ::MANAGER_STARTED_QUIESCING` which is considered as `TSTabletManager::IsOperational()` and allows apply of ops. `TSTabletManager::state_` is set to `MANAGER_STARTED_QUIESCING` at the start of `TSTabletManager::StartShutdown` and is transitioned to `MANAGER_QUIESCING` at the end. Additionally, we pre-pone the state transition to `MANAGER_RUNNING` before `OpenTablet` tasks are submitted to the threadpool. Else, this too could result in similar fatals, as `OpenTablet` would start consensus and we might trigger apply of ops even before `TSTabletManager` switches to `MANAGER_RUNNING`. **Upgrade / Downgrade safety** Added a type to `enum TSTabletManagerStatePB`. the type itself isn't being sent over the wire, so there shouldn't be any upgrade/ downgrade issues. Original commit: e524989ef94a8a0d110f5c110e2691cdb0f6859c / D51518 Test Plan: Jenkins ./yb_build.sh --cxx-test integration-tests_tablet-split-itest --gtest_filter TabletSplitITest.SplitApplyAfterTabletManagerStartShutdownBegins -n 10 --tp 1 Test fails consistently without the changes. Reviewers: sergei, esheng, arybochkin Reviewed By: esheng Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D52986
| Commit: | 95306ec | |
|---|---|---|
| Author: | Yamen Haddad | |
| Committer: | Yamen Haddad | |
[#31482] Docdb: Handle quoted identifiers in clone Summary: This change fixes owner rewriting in clone pg schema path. The diff removes the regex post processing in favor of a more robust approach where the rewrite happens withing the ysql_dump tool itself. To do that 2 new ysql_dump options where added as follows: 1- `--rename-database=new_db_name`: overrides the local datname once in `dumpDatabase()`, which transparently propagates to every `CREATE/ALTER/COMMENT/SECURITY LABEL/GRANT/REVOKE on DATABASE` and to the `\connect` line. 2- `--rename-owner=new_db_owner`: captures the source DB owner from `pg_database.datdba` inside `dumpDatabase()` and routes every OWNER TO emission through a single helper (`yb_effective_owner` in `pg_backup_archiver.c`) that substitutes the target owner only when the entry's owner equals the source DB owner. The clone path no longer needs to pass the source DB role name across the RPC chain — ysql_dump derives it itself — and the previous regex-based owner/DB-name rewriting in YsqlDumpRunner is gone. **Upgrade/Rollback safety:** Removed the `source_owner` field from the 3 RPCs used in the clone chain. The removal is wire-compatible (protobuf treats absent fields as default), but a new master sending an empty value to an old tserver corrupts owner clauses in its legacy regex-based dump rewriter and aborts the clone with a `clear role "..." does not exist error`. This only affects users during a rolling upgrade window and only for users who had explicitly enabled the EA flag `--enable_db_clone=true` on the prior release; fresh 2026.1 universes and pre-2026.1 users who haven't enabled clone in TP or EA are protected automatically because the autoflag `enable_db_clone` (kLocalPersisted) stays unpromoted until every node is on the new binary. Prioritized simplicity over dealing with this very edge case, as the mitigation is to retry cloning after fully upgrading to the new version. Test Plan: ./yb_build.sh --cxx-test integration-tests_minicluster-snapshot-test --gtest_filter PgCloneTest.CloneWithQuotedSourceOwner ./yb_build.sh --cxx-test integration-tests_minicluster-snapshot-test --gtest_filter PgCloneTest.CloneWithSpecialCharsInOwner ./yb_build.sh --cxx-test integration-tests_minicluster-snapshot-test --gtest_filter PgCloneTest.CloneRewritesOwnersOfSourceOwnedObjects ./yb_build.sh --cxx-test integration-tests_minicluster-snapshot-test --gtest_filter PgCloneTest.CloneWithBackslashInDbName Also added Java tests to test ysql_dump output: ./yb_build.sh --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpRenameDatabase' ./yb_build.sh --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpRenameOwner' ./yb_build.sh debug --java-test 'org.yb.pgsql.TestYsqlDump#ysqlDumpRenameRequiresCreate' Reviewers: zdrudi, loginov Reviewed By: zdrudi, loginov Subscribers: yql, neera.mital, hsunder, ybase Differential Revision: https://phorge.dev.yugabyte.com/D52856
| Commit: | ce7a148 | |
|---|---|---|
| Author: | jhe | |
| Committer: | jhe | |
[#31533] xCluster: Fix SIGSEGV on upgrade due to empty old_consumer_schema_versions Summary: Adding a `RepairOldSchemaVersionsPB` helper that checks for missing `old_*_schema_version` fields and backfills the empty side with a 0. This helper is run on syscatalog load and fixes the in-memory state so that we maintain the equal sized `old_*_schema_versions` lists. This doesn't persist anything to syscatalog, that will be done on the next schema version bump as `UpdateConsumerOnProducerMetadata` will fully recreate and persist these fields. Adding `[packed = false]` on `SchemaVersionsPB.old_*_schema_versions`. This restores wire-type-0 (per-element tag+varint) so a pre-D45888 reader can parse a 1-element list as the singular value. **Upgrade/Rollback safety:** `[packed = false]` only changes what the writer emits, the readers are able to accept either encoding Test Plan: ``` ybd --cxx-test xcluster_ddl_replication-test --gtest_filter 'XClusterDDLReplicationTest.RepairOldSchemaVersionsOnLoad' ``` Reviewers: hsunder, xCluster Reviewed By: hsunder Subscribers: ybase, xCluster Differential Revision: https://phorge.dev.yugabyte.com/D52960
| Commit: | f6d96a4 | |
|---|---|---|
| Author: | Anton Rybochkin | |
| Committer: | Anton Rybochkin | |
[#31350] docdb: Make admin compaction commands compact vector indexes by default Summary: Currently, the yb-ts-cli compact_tablet / compact_all_tablets commands do not consider vector indexes located on the given tablet and require an additional parameter, `--include_vector_indexes`, to compact vector indexes as well. This behavior is counterintuitive -- vector indexes should also be compacted when the corresponding compact_tablet commands are run. This change updates the behavior to include all vector indexes located on the tablet during compaction. However, since vector index compaction can be significantly heavier than RocksDB compactions of the same size, an additional parameter, `--exclude_vector_indexes`, is introduced to allow skipping vector indexes during the operation. **Upgrade/Rollback safety:** FlushCompactFlags is extended with a new item, which is simply ignored on the older version. Hence no impacts on upgrades/rollbacks. Test Plan: ./yb_build.sh --cxx-test yb-ts-cli-test --gtest_filter "YBTsCliTest.*" ./yb_build.sh --cxx-test tablet_server-test --gtest_filter "FlushAndCompact/*" Reviewers: sergei, hsunder, zdrudi Reviewed By: sergei, hsunder Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D52818
| Commit: | 41a3684 | |
|---|---|---|
| Author: | Gaurav Kukreja | |
| Committer: | Gaurav Kukreja | |
[BACKPORT 2025.2][#29313] YSQL: Use cluster configuration for geolocation costing for tables without tablespace Summary: To estimate the cost of fetching data over the network, the cost model checks the distribution of table data across the nodes. Distribution can be controlled by assigning a tablespace to the table. If no tablespace is assigned, the data is distributed according the cluster configuration. Before this change, the cost model only considered tablespace for estimating the latency and throughput costs. If no tablespace was assigned, the cost model assumed that the table was located on the same zone and estimated minimum latency and throughput costs. This is because the cluster configuration is stored in the master and not available to the cost model. A common pattern in YugabyteDB is to have a table distributed across regions and indexes with leader preference set to the zones from which the data is accessed most frequently. No tablespace is assigned to the table usually. Due to this bug, the cost model nay prefer using a primary key index over a secondary index with local leader, being ignorant to the fact that the primary key index isdistributed across regions. With this change, if tablespace is not assigned to the table,the cost model will use the cluster configuration to estimate the latency and throughput costs. Implementation details of how cluster configuration is passed to PG are as follows. The tserver caches the cluster configuration and version. Initially, the version is set to -1. In heartbeat requests to the master, the tserver sends the cluster config version. The master checks if the version is outdated, and if so, sends the updated cluster configuration and the latest version in the heartbeat response. Similarly, the cluster configuration is propagated from tserver to each local pggate instance through heartbeats. The cluster configuration is cached in pg_client, along with a snapshot which is used to ensure that the cluster configuration remains constant during the execution of a statement. At the beginning of each statement, PG asks pg_client to update the snapshot if a new cluster config is available. ## Upgrade/Downgrade safety In this change, we pass cluster `replication_info` from master to PG through tserver. Even before this change, tserver sent `cluster_config_version` to the master in the heartbeat response which was used by xcluster. With this change, if `cluster_config_version` is outdated, master will additionally send `replication_info` in the heartbeat response. During an upgrade, master nodes are upgraded first. Old tservers will simply ignore the additional `replication_info` in the heartbeat response. Original commit: eb75613a7729f52db4515aba36e25b7e7b8e3bd8 / D49700 Test Plan: ./yb_build.sh --java-test 'org.yb.pgsql.TestYbClusterConfigGeolocationCosting' Reviewers: smishra, jason, esheng, myang, mtakahara, dmitry Reviewed By: dmitry Subscribers: yql, ybase, jason, svc_phabricator Differential Revision: https://phorge.dev.yugabyte.com/D51933
| Commit: | e524989 | |
|---|---|---|
| Author: | Basava | |
| Committer: | Basava | |
[#28463] DocDB: Fix TServer crash on shutdown when concurrent raft op apply asserts TSTabletManager to be running Summary: Operations like `consensus::SPLIT_OP` and `consensus::CLONE_OP` require `TSTabletManager` to be in `MANAGER_RUNNING` state during raft apply `Operation::Replicated`. In the current code, we seem to set the shutdown state for `TSTabletManager` prior to setting shutdown state on the tablet peers. This leads to tserver crashes closer to shutdown if a split/clone op is being applied with the following log ``` F20250901 18:59:07 ../../src/yb/tablet/operations/operation_driver.cc:422] T 087c2e8412254eab823ea4dfdd3fa091 P {ts2_peer_id} S RD-P Ts { days: 20332 time: 18:59:07.479471 } kSplit (0x000072280015eaa0): Apply failed: Illegal state (yb/tserver/ts_tablet_manager.cc:1151): Manager is not running: 2, request: dest_uuid: "{ts2_peer_id}" propagated_hybrid_time: 7195660892071751680 tablet_id: "087c2e8412254eab823ea4dfdd3fa091" new_tablet1_id: "681171c81c424a1e9abd641bd10027bf" new_tablet2_id: "9b26091fb731444292ff4ed78adf15fd" split_partition_key: "44C8" split_encoded_key: "4744C8" split_parent_leader_uuid: "{ts2_peer_id}" ``` This revision addresses the above issue by introducing another state `TSTabletManagerStatePB ::MANAGER_STARTED_QUIESCING` which is considered as `TSTabletManager::IsOperational()` and allows apply of ops. `TSTabletManager::state_` is set to `MANAGER_STARTED_QUIESCING` at the start of `TSTabletManager::StartShutdown` and is transitioned to `MANAGER_QUIESCING` at the end. Additionally, we pre-pone the state transition to `MANAGER_RUNNING` before `OpenTablet` tasks are submitted to the threadpool. Else, this too could result in similar fatals, as `OpenTablet` would start consensus and we might trigger apply of ops even before `TSTabletManager` switches to `MANAGER_RUNNING`. **Upgrade / Downgrade safety** Added a type to `enum TSTabletManagerStatePB`. the type itself isn't being sent over the wire, so there shouldn't be any upgrade/ downgrade issues. Test Plan: Jenkins ./yb_build.sh --cxx-test integration-tests_tablet-split-itest --gtest_filter TabletSplitITest.SplitApplyAfterTabletManagerStartShutdownBegins -n 10 --tp 1 Test fails consistently without the changes. Reviewers: sergei, esheng, arybochkin Reviewed By: sergei, arybochkin Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D51518
| Commit: | 8156be6 | |
|---|---|---|
| Author: | Sanketh I | |
| Committer: | Sanketh I | |
[#31094] YSQL: Ability to better validate ysql_pg_conf_csv and related gflags Summary: ## Problem When a user sets a bad ysql_pg_conf_csv / ysql_hba_conf_csv / ysql_ident_conf_csv and restarts the tserver, PG fails to start up. The user has to find the error from the PG logs on the host. Currently, the gflags validation RPC only validates that these flags are comma-separated, but this is not sufficient to catch incorrect entries in hba/ident conf or invalid GUC settings (e.g., integer for boolean values). PG GUCs have internal validation functions that can assert acceptable values for a GUC. ## Summary This diff extends the ValidateFlagValue RPC to perform deeper validation of YSQL configuration gflags by writing temp config files and invoking PostgreSQL's own parsers (load_hba, load_ident, set_config_option) via a new SQL function. Each validation call typically completes in ~50ms and returns PG-native error messages including hints (e.g., "Available values: debug5, debug4, ..."). The validation writes full config files (using the same WritePostgresConfig/WritePgHbaConfig/WritePgIdentConfig code paths used at startup) to a temporary directory and asks PostgreSQL to parse them via a new SQL function yb_pg_validate_conf_file(). The intended caller is YBA & yb-ts-cli set_flag. `yb-ts-cli set_flag` now always validates flags before setting them and has a flag `--flag_validate=false` to skip the validation and set directly. ## Details New PG function and RPCs `yb_pg_validate_conf_file(hba_path text, guc_path text, ident_path text) RETURNS (hba_error text, guc_error text, ident_error text)`: Superuser-only SQL function. Validates each config file independently via PG's native parsers. NULL input skips validation for that config type; NULL output means no error. Errors from one file do not block validation of the others. `V102__31094__yb_pg_validate_conf_file.sql` adds the function to YSQL migrations. `ValidateFlagValue RPC (existing)`: The request now supports batch validation via repeated FlagValuePB flags (only one flag was supported earlier). The response uses map<string, string> errors where only flags that failed validation appear (empty map = all valid). Gflag validation is normally done through special validator hooks. But those don't work in this case because we read other flags to populate postgres config in temp files and the validator hook holds a lock that prevents this. ``` ValidateFlagValue RPC (batch: repeated FlagValuePB flags) │ │ For each flag in request: │ ├─► Basic gflag validation (existing callbacks, e.g. CSV syntax check) │ └─ Returns error immediately if basic validation fails │ ├─► For ysql_*_conf_csv flags (ysql_pg_conf_csv / ysql_hba_conf_csv / ysql_ident_conf_csv) │ │ │ ▼ │ Collect all ysql_*_conf_csv flags from the batch. │ For flags not in the request, use current running values. │ Validate all 3 config types every time. │ │ │ ▼ │ TabletServer::ValidateConfCsvViaPg() │ │ │ ├─► Create temp directory │ │ │ ├─► WritePostgresConfFiles(tmp_dir) │ │ ├─ Writes postgresql.conf from ysql_pg_conf_csv │ │ ├─ Writes pg_hba.conf from ysql_hba_conf_csv │ │ └─ Writes pg_ident.conf from ysql_ident_conf_csv │ │ │ ├─► Open internal PG connection as postgres user to "yugabyte" DB │ │ │ ├─► SELECT * FROM yb_pg_validate_conf_file(hba_path, guc_path, ident_path) │ │ │ │ │ │ Inside PG backend (yb_pg_conf_validator.c): │ │ │ │ │ ├─ GUC: ParseConfigFile() ──► set_config_option(ERROR, changeVal=false) │ │ │ each param wrapped in PG_TRY/PG_CATCH to capture full error + hint │ │ │ │ │ ├─ HBA: load_hba(hba_path) ──► PG's HBA parser + address validation │ │ │ PG_TRY/PG_CATCH captures first error │ │ │ │ │ └─ Ident: load_ident(cxt, path) ──► PG's ident map parser + regex validation │ │ PG_TRY/PG_CATCH captures first error │ │ │ │ │ └─► Returns (hba_error, guc_error, ident_error) -- NULL means success │ │ │ └─► Return errors in response.errors map │ └─► Response: map<string, string> errors (empty = all valid) ``` ## Caveats 1. Validation requires a running PG backend (internal connection to yugabyte DB). If PG is not yet up or unreachable or has hit conn limits, validation fails. 2. HBA/ident validation captures only the first error per file. ysql_pg_conf_csv validation captures all errors. **Upgrade/Rollback safety:** 1. Old proto fields for ValidateFlagRequest are still supported. Test Plan: Two C++ tests in pg_wrapper-test.cc share a unified RunConfValidationCases verifier covering GUC errors (bad types, unknown params, enum hint capture), HBA errors (bad auth methods, invalid addresses), ident errors (malformed entries, bad regexes), and multi-flag error batching. ValidateConfViaSql tests the SQL function directly; ValidateConfViaGflagValidation tests the end-to-end RPC path. ``` ./yb_build.sh fastdebug --cxx-test pg_wrapper-test --gtest_filter 'PgWrapperTest.ValidateConfViaSql:PgWrapperFlagsTest.ValidateConfVia*' ``` Reviewers: kramanathan, anijhawan Reviewed By: kramanathan, anijhawan Subscribers: smishra, yql, ybase, anijhawan Differential Revision: https://phorge.dev.yugabyte.com/D51050
| Commit: | 79c06c8 | |
|---|---|---|
| Author: | Zachary Drudi | |
| Committer: | Zachary Drudi | |
[BACKPORT 2026.1][#31278] docdb: Fix for reading sequences table as-of a time. Summary: Cloning a database with sequences can fail with SNAPSHOT_TOO_OLD when the sequences_data tablet's MVCC history has been compacted past the clone's restore time. During clone, ysql_dump reads sequence values from the live sequences_data table at the restore timestamp. On the master, AllowedHistoryCutoffProvider correctly accounts for retention_duration_sec when computing the history cutoff. However, on tservers, TabletSnapshots::AllowedHistoryCutoff() only constrains the cutoff by last_snapshot_ht_, which advances with each new snapshot. Once last_snapshot_ht_ moves past the restore time, compaction is free to GC the data the clone needs. This diff fixes the problem by propagating each snapshot schedule's retention_duration_sec from the master to tservers via the heartbeat, and using it to constrain the history cutoff on sequences_data tablets. This is scoped specifically to sequences_data to avoid unnecessary history retention (and disk bloat) on regular user tables. Note we may still run into snapshot too old errors when editing a snapshot schedule to increase its retention, and then issuing a clone. **Upgrade/Rollback safety:** Optional proto field that is only read if set at the client. Original commit: c0cecb9975251ee398bc6325c26eeb1f4e96ff01 / D52400 Test Plan: New test: PgCloneSequencesRetentionTest.CloneWithSequencesAfterCompaction Without the fix: clone fails with SNAPSHOT_TOO_OLD With the fix: clone succeeds and returns correct data ``` ./yb_build.sh release --cxx-test-filter-re minicluster-snapshot-test --cxx-test minicluster-snapshot-test --gtest_filter "PgCloneSequencesRetentionTest.CloneWithSequencesAfterCompaction" ``` Reviewers: mhaddad Reviewed By: mhaddad Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D52687
| Commit: | 7891ee9 | |
|---|---|---|
| Author: | Zachary Drudi | |
| Committer: | Zachary Drudi | |
[BACKPORT 2025.2][#31278] docdb: Fix for reading sequences table as-of a time. Summary: Cloning a database with sequences can fail with SNAPSHOT_TOO_OLD when the sequences_data tablet's MVCC history has been compacted past the clone's restore time. During clone, ysql_dump reads sequence values from the live sequences_data table at the restore timestamp. On the master, AllowedHistoryCutoffProvider correctly accounts for retention_duration_sec when computing the history cutoff. However, on tservers, TabletSnapshots::AllowedHistoryCutoff() only constrains the cutoff by last_snapshot_ht_, which advances with each new snapshot. Once last_snapshot_ht_ moves past the restore time, compaction is free to GC the data the clone needs. This diff fixes the problem by propagating each snapshot schedule's retention_duration_sec from the master to tservers via the heartbeat, and using it to constrain the history cutoff on sequences_data tablets. This is scoped specifically to sequences_data to avoid unnecessary history retention (and disk bloat) on regular user tables. Note we may still run into snapshot too old errors when editing a snapshot schedule to increase its retention, and then issuing a clone. **Upgrade/Rollback safety:** Optional proto field that is only read if set at the client. Original commit: c0cecb9975251ee398bc6325c26eeb1f4e96ff01 / D52400 Test Plan: New test: PgCloneSequencesRetentionTest.CloneWithSequencesAfterCompaction Without the fix: clone fails with SNAPSHOT_TOO_OLD With the fix: clone succeeds and returns correct data ``` ./yb_build.sh release --cxx-test-filter-re minicluster-snapshot-test --cxx-test minicluster-snapshot-test --gtest_filter "PgCloneSequencesRetentionTest.CloneWithSequencesAfterCompaction" ``` Reviewers: mhaddad Reviewed By: mhaddad Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D52707
| Commit: | 9c66fe9 | |
|---|---|---|
| Author: | Naorem Khogendro Singh | |
| Committer: | Naorem Khogendro Singh | |
[BACKPORT 2025.2][PLAT-19661][PLAT-20472][PLAT-20506][PLAT-20210] Add an automated way for Wells Fargo to change the installation directory from their custom directory to yugabyte user's home directory Summary: The following diffs are backported to reduce the conflicts and they also have some fixes. Manually tested for this branch by running locally - onprem and CSPs with custom home paths. Original diffs: 1. https://phorge.dev.yugabyte.com/D50614 (ce857a262d92c78df53cd71d11ba9b7144964734) 2. https://phorge.dev.yugabyte.com/D52003 (d335d5ddcf2d2fc3cf2e408d2c260764a43d8f38) 3. https://phorge.dev.yugabyte.com/D52247 (64fb0bbb418635658aa90eb1c32e29f69cb40bdc) 4. https://phorge.dev.yugabyte.com/D51509 (faba3e9120bae3cbfd9f931d2e765188bf95b9ea) This unifies onprem manual prechecks into YNP prechecks. If the provider is non-manual and onprem, devops prechecks are still used because there is no YNP installed. How it works: 1. Common generator shared code for YNPProvisioning, Detached prechecks and Universe node prechecks. 2. YBA <-> Node agent interacts via new RPC. YBA sends the JSON string as input. Node agent locally writes to a file and launches the precheck. 3. Stdout and stderr are sent back to YBA for observability and debugging. 4. More checks like port available checks, space checks are added. Port check is only used for this YBA triggered precheck on the first run. Manual node-agent-provision.sh cannot use due to idempotency issue. So, it is disabled in this case. 5. Disk space thresholds are still exposed on YBA that are passed to node-agent for convenience. 6. Known, fixed values like ulimits are deprecated on YBA as they are not needed anymore. [PLAT-20472] High rate of false positives from Internet Connection check impacting universe creation reliability Enable internet check only for onprem providers as in old prechecks. [PLAT-20506] Add an automated way for Wells Fargo to change the installation directory from their custom directory to yugabyte user's home directory 1. Add a separate yb_user_home for yugabyte user. 2. Use the existing yb_home_dir for software. This already works because cron works with this. All the node agent actions/tasks use this. So, no change in existing functionality. 3. Add an internal runtime config for yb_home_user override for easy tests. [PLAT-20210] Change sysctl to /usr/sbin/sysctl for prechecks run by yugabyte user To fix yugabyte@ip-10-9-192-69:~/node-agent/pkg/scripts> sysctl Absolute path to 'sysctl' is '/usr/sbin/sysctl', so running it may require superuser privileges (eg. root). when the path is not in user PATH and the binary is in sbin path. Test Plan: Manually tested the scenarios: 1. Run a python http server on 7000 and run detached precheck. It fails as expected. ``` { "check": "Port 9300 Check", "result": "FAIL", "message": "Port 9300 is unavailable on 10.9.96.53" }, { "check": "Port 7000 Check", "result": "PASS", "message": "Port 7000 is available on 10.9.96.53" }, { .... }, { "check": "Port 5433 Check", "result": "PASS", "message": "Port 5433 is open on 10.9.96.53" } ]} Pre-flight checks failed, Please fix them before continuing. ``` 2. Stop the server. It passes as expected. ``` { "check": "Port 12000 Check", "result": "PASS", "message": "Port 12000 is open on 10.9.96.53" }, { "check": "Port 9042 Check", "result": "PASS", "message": "Port 9042 is open on 10.9.96.53" }, { "check": "Port 13000 Check", "result": "PASS", "message": "Port 13000 is open on 10.9.96.53" }, { "check": "Port 5433 Check", "result": "PASS", "message": "Port 5433 is open on 10.9.96.53" } ]} Pre-flight checks successful ``` 3. Create onprem universe that internally runs non-detached precheck. It passes. 4. Create aws universe to make sure the path is not broken. It passes. 5. YNPProvisioningTest internally tests YNPConfigGenerator. All UTs pass. 6. Turn off this feature to make sure legacy check still works. It passes. 7. UI screen for failure. {F458383} Manually checked by creating a universe. 1. Manually tested with CSPs and onprem. 2. Tested generate config, config override. 3. Health checks are green. Onprem manual {F477504} CSPs {F477505} Manually tested on the VM where the issue happened. Also verified the generated script. ``` [ec2-user@ip-10-9-96-53 scripts]$ head -10 /tmp/tmp1352856859 #!/bin/bash export PATH="$PATH:/usr/sbin" ``` ``` [ec2-user@ip-10-9-96-53 scripts]$ head -n 5 /tmp/tmp3180348896 #!/bin/bash export PATH="$PATH:/usr/sbin" ``` Reviewers: anijhawan, spothuraju, skhilar, nbhatia, yshchetinin, vkumar Reviewed By: spothuraju Subscribers: hsunder, svc_phabricator, yugaware Differential Revision: https://phorge.dev.yugabyte.com/D52583
| Commit: | 5e15aa0 | |
|---|---|---|
| Author: | Gaurav Singh | |
| Committer: | Gaurav Singh | |
[BACKPORT 2025.2][#30431] YSQL,ASH: Add plan_id to ASH metadata Summary: ##SUMMARY This change adds a `plan_id` column to `yb_active_session_history(ASH)` to help diagnose **query latency** regressions caused by execution plan changes. By correlating ASH samples using (query_id, plan_id), users can verify whether a latency regression is associated with a different plan for the same normalized query. - **Queries appearing in ASH** follow `pg_stat_statements.track` (PGSS). Only top-level queries appear when PGSS=top; outer and nested queries appear when PGSS=all. - **plan_id** is controlled by `yb_pg_stat_plans_track` (QPM) for those queries that ASH tracks: - `yb_pg_stat_plans_track = 'none'`: plan_id is never set in ASH (always 0). - `yb_pg_stat_plans_track = 'top'`: plan_id is non-zero only for the outer/top-level query; nested queries have plan_id 0. - `yb_pg_stat_plans_track = 'all'`: plan_id is non-zero for every query that ASH tracks (outer and nested when PGSS=all), so you can tie ASH samples to specific plan nodes at any nesting level. **Upgrade/Rollback safety:** Upgrade is safe because `plan_id` is an optional protobuf field that defaults to 0 when absent; rollback is safe because ASH is purely in-memory with no persisted state, so the old binary simply ignores the extra column. Jira -20314 Original commit: 4dc275b75d3211d22934fa64308af868b6a3a8f9 / D50808 Test Plan: ./yb_build.sh release --cxx-test pg_ash-test --gtest_filter PgAshSingleNode.TestDmlPlanIdNonZero ./yb_build.sh release --cxx-test pg_ash-test --gtest_filter PgAshSingleNode.TestPlanIdZeroWhenQpmOff ./yb_build.sh release --cxx-test pg_ash-test --gtest_filter PgAshSingleNode.TestPlanIdExistsInStatPlans ./yb_build.sh release --cxx-test pg_ash-test --gtest_filter PgAshSingleNode.TestDifferentPlansHaveDifferentPlanIds ./yb_build.sh release --cxx-test pg_ash-test --gtest_filter PgAshNestedQueryTracking.TestNestedQueryTrackingConfigs ./yb_build.sh release --java-test 'org.yb.pgsql.TestYsqlUpgrade#migratingIsEquivalentToReinitdb' ./yb_build.sh release --cxx-test ash_metadata_upgrade-test ./yb_build.sh release --cxx-test wait_state-test Reviewers: asaha, ishan.chhangani, cagrawal, aman.mangal, #db-approvers Reviewed By: ishan.chhangani, #db-approvers Subscribers: hbhanawat, jason, yql Differential Revision: https://phorge.dev.yugabyte.com/D52658
| Commit: | c0cecb9 | |
|---|---|---|
| Author: | Zachary Drudi | |
| Committer: | Zachary Drudi | |
[#31278] docdb: Fix for reading sequences table as-of a time. Summary: Cloning a database with sequences can fail with SNAPSHOT_TOO_OLD when the sequences_data tablet's MVCC history has been compacted past the clone's restore time. During clone, ysql_dump reads sequence values from the live sequences_data table at the restore timestamp. On the master, AllowedHistoryCutoffProvider correctly accounts for retention_duration_sec when computing the history cutoff. However, on tservers, TabletSnapshots::AllowedHistoryCutoff() only constrains the cutoff by last_snapshot_ht_, which advances with each new snapshot. Once last_snapshot_ht_ moves past the restore time, compaction is free to GC the data the clone needs. This diff fixes the problem by propagating each snapshot schedule's retention_duration_sec from the master to tservers via the heartbeat, and using it to constrain the history cutoff on sequences_data tablets. This is scoped specifically to sequences_data to avoid unnecessary history retention (and disk bloat) on regular user tables. Note we may still run into snapshot too old errors when editing a snapshot schedule to increase its retention, and then issuing a clone. **Upgrade/Rollback safety:** Optional proto field that is only read if set at the client. Test Plan: New test: PgCloneSequencesRetentionTest.CloneWithSequencesAfterCompaction Without the fix: clone fails with SNAPSHOT_TOO_OLD With the fix: clone succeeds and returns correct data ``` ./yb_build.sh release --cxx-test-filter-re minicluster-snapshot-test --cxx-test minicluster-snapshot-test --gtest_filter "PgCloneSequencesRetentionTest.CloneWithSequencesAfterCompaction" ``` Reviewers: mhaddad Reviewed By: mhaddad Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D52400
| Commit: | 294f1ac | |
|---|---|---|
| Author: | jhe | |
| Committer: | jhe | |
[BACKPORT 2025.2][#30038] docdb: Enable async writes for ReadRequests that perform writes Summary: Some reads require write operations to be performed. For example SELECT FOR UPDATE (row locking) and foreign key checks. This diff makes these write operations also use the write pipelining framework. Since the writes are applied immediately on the leader, the reads can also happen quickly without waiting for the writes to be fully replicated. **Upgrade/Rollback safety:** - Only adding new fields that are only used if `ysql_enable_write_pipelining` is also enabled Original commit: 40a30c75c03872325aae98b5a7978d3aa38ae99e / D49893 Backport changes: - changed WriteResponseMsg -> WriteResponsePB Test Plan: ``` ybd --cxx-test integration-tests_async_writes-test --gtest_filter 'YSqlAsyncWriteTest.SelectForUpdateAsyncWrite' ybd --cxx-test integration-tests_async_writes-test --gtest_filter 'YSqlAsyncWriteTest.ForeignKeyAsyncWrite' ``` Reviewers: hsunder, sergei Reviewed By: hsunder Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D52446
| Commit: | 5592ccf | |
|---|---|---|
| Author: | Shishir Sharma | |
| Committer: | Shishir Sharma | |
[BACKPORT 2026.1][#28655] CDC: Remove usage of DEPRECATED_pgschema_name and fetch schema name from pg_namespace Summary: ## Backport Description: No merge conflicts ## Code Changes summary CDC records currently source `pgschema_name` from `SchemaPB::DEPRECATED_pgschema_name` which a deprecated field scheduled for removal. Once removed, CDC records would lose their schema name entirely. This revision does the following: # Removes CDC's dependency on the deprecated field and instead extends the existing `GetTableSchemaFromSysCatalog RPC` with a new `pgschema_name` response field to return the schema name fetched directly from `pg_namespace`, which is the source of truth and does not depend on any deprecated proto fields. # After deserialising the RPC response, overrides Schema::SchemaName() with the new response field's value. FillDDLInfo in cdcsdk_producer.cc now reads pgschema_name from Schema::SchemaName() instead of SchemaPB.deprecated_pgschema_name(). # Adds a LOG_IF(DFATAL, ...) guard in FillDDLInfo to catch any case where pgschema_name is unexpectedly empty. # Adds `TestAlterTableSetSchemaUpdatesSchemaNameInCDC` test that verifies `pgschema_name` is "public" before ALTER TABLE SET SCHEMA and "new_schema" after, for both DDL and INSERT records. ## Upgrade/rollback safety considerations: During rolling upgrades, masters are upgraded first, then tservers. - **New master + old tserver**: The master populates the new `pgschema_name` response field. Old tservers don't know about it and ignore it. CDC continues to work using the deprecated field until the tserver is upgraded. - **New master + new tserver**: The tserver reads the new `pgschema_name` from the response, overriding any value from the deprecated SchemaPB field - **Rollback**: No impact. Rolled-back tservers will simply ignore the new response field and resume reading from the deprecated field. ## Considerations for colocated tables: N/A. GetPgSchemaName works via `pg_class/pg_namespace` lookup using table OIDs, which applies equally to colocated and non-colocated tables. ## Considerations for connector: The gRPC connector reads `pgschema_name` from RowMessage in CDC records. This change is transparent to the connector, it receives the correct schema name without any connector-side changes. After ALTER TABLE SET SCHEMA, the connector will now see the updated schema name in subsequent CDC records (both DDL and DML records). Original commit: 6a37b828e9cd7ea0901d41df14d12bc7f7b5ef9e / D51478 Test Plan: ``` ./yb_build.sh debug --clang17 --cxx-test cdcsdk_ysql-test --gtest_filter="CDCSDKYsqlTest.TestAlterTableSetSchemaUpdatesSchemaNameInCDC" + Existing Server Unit tests + Existing Connector Unit tests ``` Reviewers: skumar, asrinivasan, devansh.saxena, sumukh.phalgaonkar, sanketh, loginov, #db-approvers Reviewed By: sumukh.phalgaonkar, #db-approvers Subscribers: ycdcxcluster, ybase, svc_phabricator Differential Revision: https://phorge.dev.yugabyte.com/D52524
| Commit: | a71f00f | |
|---|---|---|
| Author: | Devansh Singhal | |
| Committer: | Devansh Singhal | |
[BACKPORT 2024.2][#23497] CDC: Clean stale entries from cdc_state table Summary: ##### Backport Description Resolved few minor merge conflicts in cdc_service.cc, xrepl_catalog_manager.cc and cdcsdk_consistent_snapshot.cc files. Those were just related to code placement. In test file cdcsdk_consistent_snapshot.cc, conflicts came due to incoming changes which had FLAGS_ysql_yb_enable_implicit_dynamic_tables_logical_replication and FLAGS_ysql_ddl_rpc_timeout_sec in it. Removed these lines since such flags are not defined in 2024.2 (i.e their revisions are not present in 2024.2). Had a compilation error that `last_seen_tablet_stream_entries_` required a `std::hash` specialization for `TabletStreamInfo`, but none existed since `last_seen_tablet_stream_entries_` was being defined as `std::shared_ptr<std::unordered_set<TabletStreamInfo>` without specifying the hash function. As a fix, `TabletStreamInfo` already has a hash function defined so the same is provided in `last_seen_tablet_stream_entries_`'s declaration. An already existing local variable `expired_entries` in `CDCServiceImpl::UpdateMetrics()` uses the same in its declaration. This issue didn't occurred on master because on master `TabletStreamInfo` already uses the `YB_STRUCT_DEFINE_HASH` macro which automatically provide `std::hash<TabletStreamInfo>`. ##### Original Description It is seen that when a database with a replication slot is dropped followed by the stream drop, the current logic fails to delete the related tablet-stream entries as well as slot entry from the cdc_state table. This issue occurs because: - when a **DB drop** is issued, the associated tables mark the related streams with `DELETING_METADATA` state. - There's a background task which responds on `DELETING_METADATA` state and removes associated tablet-stream entries from cdc state table. - However, it won't get a chance to run if **Stream drop** is issued before its run. This is because **stream delete** workflow overwrites stream state to `DELETING`. - The other background task which responds on `DELETING` state updates the checkpoint of associated tablet stream entries and slot entry to max. - UpdatePeersAndMetrics() (aka UPAM) tries to delete the entries which have max checkpoint. But it first tries to update all the associated tablet peers about the max checkpoint (so that each tablet peers can remove the retention barriers and so release their resources (such as WAL logs, Intent's SST files, History SST files)). It fails here since the tablet peers are already gone (as part of table deletion). - This way such entries always linger in cdc state table. This diff solves this issue in 2 sub-solutions: 1. It restricts moving stream to any other state if stream is already in `DELETING` (this is so because for example overwriting state DELETING to DELETING_METADATA may cause stale slot entry to remain in state table). 2. It lets all the workflows, which were previously setting the concerned cdc stable table entries checkpoint to OpId::Max() (so that UPAM get to know about such entries and can update peers about max checkpoint and then delete such entries), instead delete such entries rightaway. This is because diff removes the restrictions from UPAM to update peers about max checkpoint. The retention barriers on peers will eventually go stale and associated resources will be released using a newly introduced maintenance op i.e. `ResetStaleRetentionBarriersOp`. - This op checks if the barriers have gone stale (using `TabletPeer::is_cdc_min_replicated_index_stale()`) and if this op had ran before the refresh of `cdc_min_replicated_index`. If so, then it shows its interest of execution to `MaintenanceManager`. - Once it executes, it resets all cdc retention barriers using `TabletPeer::reset_all_cdc_retention_barriers_if_stale()`. **Note that** we are still keeping the logic of deleting entries with max checkpoint in UPAM so as to ensure that all stale entries gets deleted if some workflows still updating entries' checkpoint to OpId::Max(). - For removal of tablet metrics for a stream which are not required, we now take a set difference of tablet-stream entries present in cdc state table during previous iteration of `CDCServiceImpl::UpdateMetrics()` vs now. If some of the tablet-stream entries are missing in latest iteration, then metrics for such stream is removed from concerned tablet. for this, we use a newly introduced in-mem variable `last_seen_tablet_stream_entries_` instead of `CDCServiceImpl::Impl's tablet_checkpoints_` (This is necessary because `tablet_checkpoints_` insert tablet-stream elements only during a GetChanges call, so there might be some tablets which are never polled and so the previous logic fails to remove metrics for such tablets). The diff also changes the default value of gflag "cdc_min_replicated_index_considered_stale_secs" from 15 mins to 30 mins. Upgrade/Rollback safety: - Diff adds the repeated field `deleted_tablet_entries` and deprecates field `updated_tablet_entries` in `ValidateAndSyncCDCStateEntriesForCDCSDKStreamResponsePB` message in `master_replication.proto`: This is not a concern for upgrades and rollback since the above message is being used for `yb-admin`'s CDCSDK cmd `validate_and_sync_cdc_state_table_entries_on_change_data_stream`. A node (on older version) consuming the response from node (on newer version) will discard the newer field values making the output of cmd incomplete. However, internally, the data and its state remains consistent. - Diff also add optional field `cdcsdk_reset_stale_retention_barrier` in message `MaintenanceOpPB` in `tablet.proto`: This is also okay from upgrade and rollback perspective since the producer and consumer of this proto message lies on same node. Jira: DB-12410 Original commit: 1ef98f6fa4a9e4ff1e4f47218a36de78ae93a504 / D45461 Test Plan: The diff runs all tests as part of test plan. yb_build.sh --cxx-test cdcsdk_consumption_consistent_changes-test --gtest_filter=CDCSDKConsumptionConsistentChangesTest.TestRetentionBarrierPreservedUntilStale yb_build.sh --cxx-test cdcsdk_consumption_consistent_changes-test --gtest_filter=CDCSDKConsumptionConsistentChangesTest.CheckSlotRowDeletionForStreamAndTableDeletion yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestMetricObjectRemovalAfterNamespaceDeletion yb_build.sh --cxx-test integration-tests_cdcsdk_consistent_snapshot-test --gtest_filter CDCSDKConsistentSnapshotTest.TestRetentionBarrierSettingRace yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestValidationAndSyncOfCDCStateEntriesAfterUserTableRemovalOnNonConsistentSnapshotStream yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestValidationAndSyncOfCDCStateEntriesAfterUserTableRemovalOnConsistentSnapshotStream yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestDeletedStreamRowsRemoved yb_build.sh --cxx-test maintenance_manager-test --gtest_filter=MaintenanceManagerTest.TestRegisterUnregister yb_build.sh --cxx-test maintenance_manager-test --gtest_filter=MaintenanceManagerTest.TestCompletedOpsHistory Reviewers: sumukh.phalgaonkar, skumar, stiwary, asrinivasan, xCluster, hsunder Reviewed By: sumukh.phalgaonkar Subscribers: ycdcxcluster, svc_phabricator, ybase, schandra Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D52432
| Commit: | 22e337c | |
|---|---|---|
| Author: | Devansh Singhal | |
| Committer: | Devansh Singhal | |
[BACKPORT 2025.1][#23497] CDC: Clean stale entries from cdc_state table Summary: ##### Backport Description Resolved few minor merge conflicts in cdc_service.h/.cc, tablet_peer_mm_ops.cc and cdcsdk_ysql_test_base.cc files. Those were just related to code placement. ##### Original Description It is seen that when a database with a replication slot is dropped followed by the stream drop, the current logic fails to delete the related tablet-stream entries as well as slot entry from the cdc_state table. This issue occurs because: - when a **DB drop** is issued, the associated tables mark the related streams with `DELETING_METADATA` state. - There's a background task which responds on `DELETING_METADATA` state and removes associated tablet-stream entries from cdc state table. - However, it won't get a chance to run if **Stream drop** is issued before its run. This is because **stream delete** workflow overwrites stream state to `DELETING`. - The other background task which responds on `DELETING` state updates the checkpoint of associated tablet stream entries and slot entry to max. - UpdatePeersAndMetrics() (aka UPAM) tries to delete the entries which have max checkpoint. But it first tries to update all the associated tablet peers about the max checkpoint (so that each tablet peers can remove the retention barriers and so release their resources (such as WAL logs, Intent's SST files, History SST files)). It fails here since the tablet peers are already gone (as part of table deletion). - This way such entries always linger in cdc state table. This diff solves this issue in 2 sub-solutions: 1. It restricts moving stream to any other state if stream is already in `DELETING` (this is so because for example overwriting state DELETING to DELETING_METADATA may cause stale slot entry to remain in state table). 2. It lets all the workflows, which were previously setting the concerned cdc stable table entries checkpoint to OpId::Max() (so that UPAM get to know about such entries and can update peers about max checkpoint and then delete such entries), instead delete such entries rightaway. This is because diff removes the restrictions from UPAM to update peers about max checkpoint. The retention barriers on peers will eventually go stale and associated resources will be released using a newly introduced maintenance op i.e. `ResetStaleRetentionBarriersOp`. - This op checks if the barriers have gone stale (using `TabletPeer::is_cdc_min_replicated_index_stale()`) and if this op had ran before the refresh of `cdc_min_replicated_index`. If so, then it shows its interest of execution to `MaintenanceManager`. - Once it executes, it resets all cdc retention barriers using `TabletPeer::reset_all_cdc_retention_barriers_if_stale()`. **Note that** we are still keeping the logic of deleting entries with max checkpoint in UPAM so as to ensure that all stale entries gets deleted if some workflows still updating entries' checkpoint to OpId::Max(). - For removal of tablet metrics for a stream which are not required, we now take a set difference of tablet-stream entries present in cdc state table during previous iteration of `CDCServiceImpl::UpdateMetrics()` vs now. If some of the tablet-stream entries are missing in latest iteration, then metrics for such stream is removed from concerned tablet. for this, we use a newly introduced in-mem variable `last_seen_tablet_stream_entries_` instead of `CDCServiceImpl::Impl's tablet_checkpoints_` (This is necessary because `tablet_checkpoints_` insert tablet-stream elements only during a GetChanges call, so there might be some tablets which are never polled and so the previous logic fails to remove metrics for such tablets). The diff also changes the default value of gflag "cdc_min_replicated_index_considered_stale_secs" from 15 mins to 30 mins. Upgrade/Rollback safety: - Diff adds the repeated field `deleted_tablet_entries` and deprecates field `updated_tablet_entries` in `ValidateAndSyncCDCStateEntriesForCDCSDKStreamResponsePB` message in `master_replication.proto`: This is not a concern for upgrades and rollback since the above message is being used for `yb-admin`'s CDCSDK cmd `validate_and_sync_cdc_state_table_entries_on_change_data_stream`. A node (on older version) consuming the response from node (on newer version) will discard the newer field values making the output of cmd incomplete. However, internally, the data and its state remains consistent. - Diff also add optional field `cdcsdk_reset_stale_retention_barrier` in message `MaintenanceOpPB` in `tablet.proto`: This is also okay from upgrade and rollback perspective since the producer and consumer of this proto message lies on same node. Jira: DB-12410 Original commit: 1ef98f6fa4a9e4ff1e4f47218a36de78ae93a504 / D45461 Test Plan: The diff runs all tests as part of test plan. yb_build.sh --cxx-test cdcsdk_consumption_consistent_changes-test --gtest_filter=CDCSDKConsumptionConsistentChangesTest.TestRetentionBarrierPreservedUntilStale yb_build.sh --cxx-test cdcsdk_consumption_consistent_changes-test --gtest_filter=CDCSDKConsumptionConsistentChangesTest.CheckSlotRowDeletionForStreamAndTableDeletion yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestMetricObjectRemovalAfterNamespaceDeletion yb_build.sh --cxx-test integration-tests_cdcsdk_consistent_snapshot-test --gtest_filter CDCSDKConsistentSnapshotTest.TestRetentionBarrierSettingRace yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestValidationAndSyncOfCDCStateEntriesAfterUserTableRemovalOnNonConsistentSnapshotStream yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestValidationAndSyncOfCDCStateEntriesAfterUserTableRemovalOnConsistentSnapshotStream yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestDeletedStreamRowsRemoved yb_build.sh --cxx-test maintenance_manager-test --gtest_filter=MaintenanceManagerTest.TestRegisterUnregister yb_build.sh --cxx-test maintenance_manager-test --gtest_filter=MaintenanceManagerTest.TestCompletedOpsHistory Reviewers: sumukh.phalgaonkar, skumar, stiwary, asrinivasan, xCluster, hsunder Reviewed By: sumukh.phalgaonkar Subscribers: schandra, ybase, svc_phabricator, ycdcxcluster Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D52431
| Commit: | 2dd84f5 | |
|---|---|---|
| Author: | Devansh Singhal | |
| Committer: | Devansh Singhal | |
[BACKPORT 2025.2][#23497] CDC: Clean stale entries from cdc_state table Summary: ##### Backport Description No merge conflicts. ##### Original Description It is seen that when a database with a replication slot is dropped followed by the stream drop, the current logic fails to delete the related tablet-stream entries as well as slot entry from the cdc_state table. This issue occurs because: - when a **DB drop** is issued, the associated tables mark the related streams with `DELETING_METADATA` state. - There's a background task which responds on `DELETING_METADATA` state and removes associated tablet-stream entries from cdc state table. - However, it won't get a chance to run if **Stream drop** is issued before its run. This is because **stream delete** workflow overwrites stream state to `DELETING`. - The other background task which responds on `DELETING` state updates the checkpoint of associated tablet stream entries and slot entry to max. - UpdatePeersAndMetrics() (aka UPAM) tries to delete the entries which have max checkpoint. But it first tries to update all the associated tablet peers about the max checkpoint (so that each tablet peers can remove the retention barriers and so release their resources (such as WAL logs, Intent's SST files, History SST files)). It fails here since the tablet peers are already gone (as part of table deletion). - This way such entries always linger in cdc state table. This diff solves this issue in 2 sub-solutions: 1. It restricts moving stream to any other state if stream is already in `DELETING` (this is so because for example overwriting state DELETING to DELETING_METADATA may cause stale slot entry to remain in state table). 2. It lets all the workflows, which were previously setting the concerned cdc stable table entries checkpoint to OpId::Max() (so that UPAM get to know about such entries and can update peers about max checkpoint and then delete such entries), instead delete such entries rightaway. This is because diff removes the restrictions from UPAM to update peers about max checkpoint. The retention barriers on peers will eventually go stale and associated resources will be released using a newly introduced maintenance op i.e. `ResetStaleRetentionBarriersOp`. - This op checks if the barriers have gone stale (using `TabletPeer::is_cdc_min_replicated_index_stale()`) and if this op had ran before the refresh of `cdc_min_replicated_index`. If so, then it shows its interest of execution to `MaintenanceManager`. - Once it executes, it resets all cdc retention barriers using `TabletPeer::reset_all_cdc_retention_barriers_if_stale()`. **Note that** we are still keeping the logic of deleting entries with max checkpoint in UPAM so as to ensure that all stale entries gets deleted if some workflows still updating entries' checkpoint to OpId::Max(). - For removal of tablet metrics for a stream which are not required, we now take a set difference of tablet-stream entries present in cdc state table during previous iteration of `CDCServiceImpl::UpdateMetrics()` vs now. If some of the tablet-stream entries are missing in latest iteration, then metrics for such stream is removed from concerned tablet. for this, we use a newly introduced in-mem variable `last_seen_tablet_stream_entries_` instead of `CDCServiceImpl::Impl's tablet_checkpoints_` (This is necessary because `tablet_checkpoints_` insert tablet-stream elements only during a GetChanges call, so there might be some tablets which are never polled and so the previous logic fails to remove metrics for such tablets). The diff also changes the default value of gflag "cdc_min_replicated_index_considered_stale_secs" from 15 mins to 30 mins. Upgrade/Rollback safety: - Diff adds the repeated field `deleted_tablet_entries` and deprecates field `updated_tablet_entries` in `ValidateAndSyncCDCStateEntriesForCDCSDKStreamResponsePB` message in `master_replication.proto`: This is not a concern for upgrades and rollback since the above message is being used for `yb-admin`'s CDCSDK cmd `validate_and_sync_cdc_state_table_entries_on_change_data_stream`. A node (on older version) consuming the response from node (on newer version) will discard the newer field values making the output of cmd incomplete. However, internally, the data and its state remains consistent. - Diff also add optional field `cdcsdk_reset_stale_retention_barrier` in message `MaintenanceOpPB` in `tablet.proto`: This is also okay from upgrade and rollback perspective since the producer and consumer of this proto message lies on same node. Jira: DB-12410 Original commit: 1ef98f6fa4a9e4ff1e4f47218a36de78ae93a504 / D45461 Test Plan: The diff runs all tests as part of test plan. yb_build.sh --cxx-test cdcsdk_consumption_consistent_changes-test --gtest_filter=CDCSDKConsumptionConsistentChangesTest.TestRetentionBarrierPreservedUntilStale yb_build.sh --cxx-test cdcsdk_consumption_consistent_changes-test --gtest_filter=CDCSDKConsumptionConsistentChangesTest.CheckSlotRowDeletionForStreamAndTableDeletion yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestMetricObjectRemovalAfterNamespaceDeletion yb_build.sh --cxx-test integration-tests_cdcsdk_consistent_snapshot-test --gtest_filter CDCSDKConsistentSnapshotTest.TestRetentionBarrierSettingRace yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestValidationAndSyncOfCDCStateEntriesAfterUserTableRemovalOnNonConsistentSnapshotStream yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestValidationAndSyncOfCDCStateEntriesAfterUserTableRemovalOnConsistentSnapshotStream yb_build.sh --cxx-test integration-tests_cdcsdk_ysql-test --gtest_filter CDCSDKYsqlTest.TestDeletedStreamRowsRemoved yb_build.sh --cxx-test maintenance_manager-test --gtest_filter=MaintenanceManagerTest.TestRegisterUnregister yb_build.sh --cxx-test maintenance_manager-test --gtest_filter=MaintenanceManagerTest.TestCompletedOpsHistory Reviewers: sumukh.phalgaonkar, skumar, stiwary, asrinivasan, xCluster, hsunder Reviewed By: sumukh.phalgaonkar Subscribers: ycdcxcluster, svc_phabricator, ybase, schandra Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D52430
| Commit: | 6a37b82 | |
|---|---|---|
| Author: | Shishir Sharma | |
| Committer: | Shishir Sharma | |
[#28655] CDC: Remove usage of DEPRECATED_pgschema_name and fetch schema name from pg_namespace Summary: ## Code Changes summary CDC records currently source `pgschema_name` from `SchemaPB::DEPRECATED_pgschema_name` which a deprecated field scheduled for removal. Once removed, CDC records would lose their schema name entirely. This revision does the following: # Removes CDC's dependency on the deprecated field and instead extends the existing `GetTableSchemaFromSysCatalog RPC` with a new `pgschema_name` response field to return the schema name fetched directly from `pg_namespace`, which is the source of truth and does not depend on any deprecated proto fields. # After deserialising the RPC response, overrides Schema::SchemaName() with the new response field's value. FillDDLInfo in cdcsdk_producer.cc now reads pgschema_name from Schema::SchemaName() instead of SchemaPB.deprecated_pgschema_name(). # Adds a LOG_IF(DFATAL, ...) guard in FillDDLInfo to catch any case where pgschema_name is unexpectedly empty. # Adds `TestAlterTableSetSchemaUpdatesSchemaNameInCDC` test that verifies `pgschema_name` is "public" before ALTER TABLE SET SCHEMA and "new_schema" after, for both DDL and INSERT records. ## Upgrade/rollback safety considerations: During rolling upgrades, masters are upgraded first, then tservers. - **New master + old tserver**: The master populates the new `pgschema_name` response field. Old tservers don't know about it and ignore it. CDC continues to work using the deprecated field until the tserver is upgraded. - **New master + new tserver**: The tserver reads the new `pgschema_name` from the response, overriding any value from the deprecated SchemaPB field - **Rollback**: No impact. Rolled-back tservers will simply ignore the new response field and resume reading from the deprecated field. ## Considerations for colocated tables: N/A. GetPgSchemaName works via `pg_class/pg_namespace` lookup using table OIDs, which applies equally to colocated and non-colocated tables. ## Considerations for connector: The gRPC connector reads `pgschema_name` from RowMessage in CDC records. This change is transparent to the connector, it receives the correct schema name without any connector-side changes. After ALTER TABLE SET SCHEMA, the connector will now see the updated schema name in subsequent CDC records (both DDL and DML records). Test Plan: ``` ./yb_build.sh debug --clang17 --cxx-test cdcsdk_ysql-test --gtest_filter="CDCSDKYsqlTest.TestAlterTableSetSchemaUpdatesSchemaNameInCDC" + Existing Server Unit tests + Existing Connector Unit tests ``` Reviewers: skumar, asrinivasan, devansh.saxena, sumukh.phalgaonkar, sanketh, loginov Reviewed By: skumar, asrinivasan, loginov Subscribers: svc_phabricator, ybase, ycdcxcluster Differential Revision: https://phorge.dev.yugabyte.com/D51478
| Commit: | 0e83cdd | |
|---|---|---|
| Author: | Eric Sheng | |
| Committer: | Eric Sheng | |
[#30163] docdb: Add per-database thread-pools for servicing RPCs Summary: As part of the multitenancy work, all threads doing processing for a database should be in the cgroup for that database, so that we can apply CPU limits at the database level. This change handles the threads servicing a YSQL request, by having per-database thread pools and having ServicePool pick the appropriate thread pool based on a new `pool_tag` in the RPC header. This `pool_tag` is only set when the `enable_qos` gflag is on -- when the featue is disabled, it defaults to `pool_tag = 0`, which results in all requests handled on the "default" thread pool, similar to existing behavior today. It additionally is only set for YSQL operations. Handling YCQL/YEDIS requests is not planned. We use the approach of having multiple thread pools instead of setting the cgroup at the start of a request, and resetting it at the end of the request, because setting cgroups frequently was observed to have significant performance impact on short-running queries (>30% overhead for point selects). This is in-line with recommendations from kernel documentation that cgroups should be set once during setup rather than frequently switched between. **Detailed changes:** This change adds a new field, `pool_tag`, to the RPC header. This field is usually not set, and defaults to `0`. It is only set (to the database oid) when the `enable_qos` gflag is enabled, and only for subclasses of `YBPgsqlOp`. The normal priority service pool in the RPC messenger has been replaced with a set of thread pools, keyed by pool tag, with the default case being the pool corresponding with `pool_tag = 0`. When the `pool_tag` field is set in the RPC header, ServicePoolImpl::Process will pick a thread from the thread pool corresponding to the pool tag instead of the default (`pool_tag = 0`) pool to service the request. If no thread pool corresponding to the pool tag exists (e.g., because this is the first request with such a pool tag), a thread pool is started for the pool tag. **Upgrade/Rollback safety:** A new field (`pool_tag`) was added to the RPC header. An autoflag (`enable_rpc_pool_tags`) was added, and the field is only set if the autoflag is true. This is necessary because unlike most protobufs, we explicitly check for unknown fields for the RPC header, so we cannot send RPCs with this field set until the upgrade is finalized. Test Plan: `./yb_build.sh --cxx-test pg_cgroups-test --gtest_filter 'PgCgroupsTest.TestQosRead'` `./yb_build.sh --cxx-test pg_cgroups-test --gtest_filter 'PgCgroupsTest.TestQosWrite'` Reviewers: sergei Reviewed By: sergei Subscribers: amitanand, yql, svc_phabricator, ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D50829
| Commit: | 9f0f237 | |
|---|---|---|
| Author: | Sergei Politov | |
| Committer: | Sergei Politov | |
[#29787] DocDB: Use lightweight protobufs for read and write requests Summary: This diff switches read and write requests to use lightweight protobufs. Test comparison using newly added PgPerfTest.InsertStrings agains master (69e7f2a1ae24061f6b793f23c7d9f16641dc5a20) at n2-standard-4: master, full insert time: **5.96s**, accumulated tserver write time: **12.14s** this diff: full insert time: **5.68s**, accumulated tserver write time: **10.53s** I.e. 5% improvement in full time, 15% improvement in tserver time. **Upgrade / Rollback safety** Changed only the specification for method signatures, i.e. lightweight vs vanilla protobuf. It does not have effect on serialised representation. Test Plan: Jenkins Reviewers: arybochkin, xCluster, hsunder Reviewed By: arybochkin Subscribers: ybase, yql Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D47605