[CDCSDK] Move CreateCDCStreamForNamespace logic outside of cdc_service into yb-master
#18890
Closed
1 task done
Assignees
Labels
area/cdcsdk
CDC SDK
kind/enhancement
This is an enhancement of an existing feature
priority/medium
Medium priority issue
Comments
yugabyte-ci
added
the
kind/enhancement
dr0pdb
added a commit
that referenced
this issue
Oct 27, 2023
…to YB-Master
Summary:
Now that yb-master supports creating CDCSDK streams atomically, forward the CreateCDCStream requests to yb-master.
This change is guarded by the auto flag `ysql_yb_enable_replication_commands` since we need to ensure that yb-master has the upgraded code before forwarding the request to
it.
To be able to do this, this diff temporarily removes the restriction that each CreateCDCStream request for a PGSQL database must also have a replication slot name. This restriction will be added back once we've ensured that YSQL layer is the only client making the requests. This change is needed for the consistent snapshot project which will go before the publication/replication slot API.
Also fixed an issue with the implementation in yb-master. The `last_replication_time` for entries in the cdc_state_table do not need to be populated for CDCSDK streams as that is used to know whether a client has started streaming or not. It affects our handling of tablet splits.
**Upgrade/Rollback safety:**
This change adds a new error code field to an enum.
This does not need an autoflag since this is a yb-admin - cdc service RPC. Reference ("When do I not need an AutoFlag?" section): https://docs.google.com/document/d/1aFM0NPimXyrFoTGnnspjaaxZdeRBESG332zwpFsPh3A/edit#heading=h.cx5lth8w95ye
Jira: DB-7751
Test Plan:
New test
```
./yb_build.sh --cxx-test cdcsdk_ysql-test --gtest_filter "*TestCDCStreamCreationDisabledDuringUpgrade*"
./yb_build.sh --cxx-test master_xrepl-test --gtest_filter MasterTestXRepl.TestCreateCDCStreamForNamespaceDisabled
```
Existing CDCSDK tests
Reviewers: skumar, asrinivasan, hsunder
Reviewed By: asrinivasan
Subscribers: ybase, ycdcxcluster, bogdan
Tags: #jenkins-ready
Differential Revision: https://phorge.dev.yugabyte.com/D29420
Spikatrix
pushed a commit
to Spikatrix/yugabyte-db
that referenced
this issue
Nov 21, 2023
…mands a TEST flag Summary: This revision makes the already existing autoflag `yb_enable_replication_commands` a `TEST` flag. This is safe to do because the revision https://phorge.dev.yugabyte.com/D28721 that introduced the flag is only on the master branch. The flag should have been a 'TEST' flag from the start since we want to make further changes before enabling the replication slots feature by default. Now the behavior of `CreateCDCStream` RPC in yb-master is as follows: 1. `yb_enable_replication_commands` - true - YSQL commands for replication slots - supported and the namespace id is read from the `namespace_id` field - CDC service (yb-admin) - supported and the namespace id is read from the `namespace_id` field 2. `yb_enable_replication_commands` - false - YSQL commands - disallowed - CDC service (yb-admin) - supported using the old mechanism of reading the namespace id from the `table_id` field YSQL commands are identified with the presence of the `cdcsdk_ysql_replication_slot_name` in the request. As part of this work, we also cleanup the `kNamespaceId` mode from the `CreateNewCDCStreamMode` since the cdc_service just forwards the request to yb-master now, hence we no longer to support the older flow. Jira: DB-8008, DB-7751 Test Plan: New tests ``` ./yb_build.sh --cxx-test cdcsdk_ysql-test --gtest_filter "*TestCDCStreamCreationViaCDCServiceWithReplicationCommands*" ``` Existing CDC and master_xrepl tests Reviewers: asrinivasan, skumar, hsunder, xCluster Reviewed By: hsunder Subscribers: yql, ybase, ycdcxcluster, bogdan Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D30234
dr0pdb
added a commit
that referenced
this issue
Dec 14, 2023
…CDCSDK streams to YB-Master Summary: **Backport Description** The merge was clean but the revision contains a few more changes that are taken from the https://phorge.dev.yugabyte.com/D30234. These manual changes are required since in the master branch, we earlier declared the flag as an Autoflag and later changed it to a TEST flag. In this 2.20 branch, we want to start with a TEST flag itself. The following manual changes were made: 1. xrepl_catalog_manager.cc - Added `ValidateCDCSDKRequestProperties` function. Updated the code to how it was after https://phorge.dev.yugabyte.com/D30234. 2. src/yb/master/master_xrepl-test.cc - Minor conflicts due to the flag names `ysql_yb_enable_replication_commands` (AutoFlag) and `TEST_ysql_yb_enable_replication_commands` (renamed TEST flag) **Original Description** Original commit: b1727f8 / D29420 Now that yb-master supports creating CDCSDK streams atomically, forward the CreateCDCStream requests to yb-master. This change is guarded by the auto flag `ysql_yb_enable_replication_commands` since we need to ensure that yb-master has the upgraded code before forwarding the request to it. To be able to do this, this diff temporarily removes the restriction that each CreateCDCStream request for a PGSQL database must also have a replication slot name. This restriction will be added back once we've ensured that YSQL layer is the only client making the requests. This change is needed for the consistent snapshot project which will go before the publication/replication slot API. Also fixed an issue with the implementation in yb-master. The `last_replication_time` for entries in the cdc_state_table do not need to be populated for CDCSDK streams as that is used to know whether a client has started streaming or not. It affects our handling of tablet splits. **Upgrade/Rollback safety:** This change adds a new error code field to an enum. This does not need an autoflag since this is a yb-admin - cdc service RPC. Reference ("When do I not need an AutoFlag?" section): https://docs.google.com/document/d/1aFM0NPimXyrFoTGnnspjaaxZdeRBESG332zwpFsPh3A/edit#heading=h.cx5lth8w95ye Jira: DB-7751 Test Plan: Jenkins: test regex: .*CDCSDK.* New test ``` ./yb_build.sh --cxx-test cdcsdk_ysql-test --gtest_filter "*TestCDCStreamCreationDisabledDuringUpgrade*" ./yb_build.sh --cxx-test master_xrepl-test --gtest_filter MasterTestXRepl.TestCreateCDCStreamForNamespaceDisabled ``` Existing CDCSDK tests Reviewers: skumar, asrinivasan, hsunder, xCluster Reviewed By: skumar Subscribers: bogdan, ycdcxcluster, ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D31022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Jira Link: DB-7751
Description
The
CreateCDCStreamForNamespacefunction insidecdc_service.ccis responsible for a creating a CDCSDK stream. It calls various RPCs onMasterReplicationServiceto achieve that. It can be moved insideMasterReplicationServiceitself in yb-master.As part of #18724, we are moving some of the logic to yb-master but CQL CDCSDK streams will still go via
CreateCDCStreamForNamespaceflow.We need to do the following:
CreateCDCStreamto also handle CQL streams. It should just work IMO but we should confirm that. Some tests should also be added tosrc/yb/master/master_xrepl-test.ccfor CQL namespaces.Source connector version
N/A
Connector configuration
N/A
YugabyteDB version
N/A
Warning: Please confirm that this issue does not contain any sensitive information
The text was updated successfully, but these errors were encountered: