Conversation
andy-stark-redis
left a comment
There was a problem hiding this comment.
Quite a few suggestions but they are easily applied, so I'll approve to avoid delays.
…vs-flink.md Co-authored-by: andy-stark-redis <164213578+andy-stark-redis@users.noreply.github.com>
…e-dbs/_index.md Co-authored-by: andy-stark-redis <164213578+andy-stark-redis@users.noreply.github.com>
…e-dbs/snowflake.md Co-authored-by: andy-stark-redis <164213578+andy-stark-redis@users.noreply.github.com>
…e-dbs/snowflake.md Co-authored-by: andy-stark-redis <164213578+andy-stark-redis@users.noreply.github.com>
…e-dbs/spanner.md Co-authored-by: andy-stark-redis <164213578+andy-stark-redis@users.noreply.github.com>
…e-dbs/spanner.md Co-authored-by: andy-stark-redis <164213578+andy-stark-redis@users.noreply.github.com>
Co-authored-by: andy-stark-redis <164213578+andy-stark-redis@users.noreply.github.com>
Co-authored-by: andy-stark-redis <164213578+andy-stark-redis@users.noreply.github.com>
Co-authored-by: andy-stark-redis <164213578+andy-stark-redis@users.noreply.github.com>
Co-authored-by: andy-stark-redis <164213578+andy-stark-redis@users.noreply.github.com>
Co-authored-by: andy-stark-redis <164213578+andy-stark-redis@users.noreply.github.com>
Co-authored-by: andy-stark-redis <164213578+andy-stark-redis@users.noreply.github.com>
Co-authored-by: andy-stark-redis <164213578+andy-stark-redis@users.noreply.github.com>
Co-authored-by: andy-stark-redis <164213578+andy-stark-redis@users.noreply.github.com>
Co-authored-by: andy-stark-redis <164213578+andy-stark-redis@users.noreply.github.com>
…e-dbs/mongodb.md Co-authored-by: Zdravko Donev <zdravko.donev@redis.com>
zhivko-kirishev-redis
left a comment
There was a problem hiding this comment.
Everything about the automatic population of the ssl properties as well as the default Flink processor looks good
nicoweidner
left a comment
There was a problem hiding this comment.
I left a few comments. We can tackle them in a followup so as not to block the release, none of them are critical
|
|
||
| ```text | ||
| dlq:data:{rdi}:public.users | ||
| dlq:data:{rdi}:<source>.<schema_or_database>.<table> |
There was a problem hiding this comment.
Is this form true for SQLServer and Snowflake? IIRC from some review those two were exceptions and had a three-part fully qualified table name even without the source component
There was a problem hiding this comment.
Changed <schema_or_database>.<table> to <qualified_table_name> in multiple places and clarified what it means. It only affects SQLServer, Snowflake was 3-segment but will be 2-segment after https://github.com/RedisLabs/redis-data-integration/pull/2761 is merged.
BTW, this comment revealed an issue with the new reset impl for SQLServer, that I am now fixing. Thanks!
|
|
||
| {{< note >}}The scope-prefixed keys `SOURCE_DB_*` and `TARGET_DB_*` are also accepted, and can be | ||
| used without specifying `--db`. RDI accepts a `SOURCE_DB_*` key only for a pipeline with | ||
| exactly one source created before RDI supported multiple sources. See |
There was a problem hiding this comment.
I don't think the "created before RDI supported multiple sources" part is correct, we'll accept it as long as the pipeline has a single source. But maybe we can add a note that they are considered deprecated, to discourage their use?
There was a problem hiding this comment.
Removed "created before RDI supported multiple sources" but stopped short of declaring them deprecated, instead only phrased it that the new syntax should be preferred.
| | Environment variable prefix | `MYSQL_DB_` | | ||
| | Certificate directory | `/etc/certificates/mysql_db/` | | ||
| | Collector deployment | `collector-mysql` | | ||
| | Change data streams | `data:{rdi}:mysql.<schema_or_database>.<table>` | |
There was a problem hiding this comment.
This is incorrect for SQLServer which uses data:{rdi}:<source>.<database>.<schema>.<table>. Same for DLQ keys. We could make it more universal by writing dlq:data:{rdi}:<source>.<qualified_table_name>, wdyt?
There was a problem hiding this comment.
Addressed, see above.
| the reset runs and starts it again afterwards, exactly as it does for a reset of the whole | ||
| pipeline. | ||
|
|
||
| Data that is not partitioned by source is never deleted by a per-source reset. |
There was a problem hiding this comment.
I don't understand this sentence. Does it refer to any metadata in the RDI DB that is not per-source data?
I'd suggest clarifying which data we mean here, or potentially dropping it entirely, as the previous paragraph already lists which data is deleted.
There was a problem hiding this comment.
Removed. This was about the classic processor rdi:applier and rdi:batcher keys. The content of these keys is not "partitioned per source" and therefore never deleted by a per-source reset. But you are right the original sentence is obscure, and clarifying it would mean exposing low-level details of a processor that we are in the process of deprecating.
| [Accessing the metrics]({{< relref "/integrate/redis-data-integration/observability#accessing-the-metrics" >}}). | ||
|
|
||
| Dead-letter queue streams have Redis keys containing a | ||
| `<source>.<schema_or_database>.<table>` section. |
There was a problem hiding this comment.
Same comment regarding the key format for SQLServer applies here
There was a problem hiding this comment.
Addressed, see above.
| schemas: | ||
| - public | ||
| tables: | ||
| public.customers: {} |
There was a problem hiding this comment.
A review agent found that this example has a potential target key collision, since the default key generator will use e.g. customers:id:1 if the PK is id for both sources. While I think this misses the point of the example, we could still clear any potential confusion by renaming the second table to something else like public.clients
| When a pipeline has more than one source, every job must set `server_name`, and the value | ||
| must match one of the sources in `config.yaml`. RDI rejects the pipeline when a job has no |
There was a problem hiding this comment.
An aliased source retains server_name: rdi; we should probably point out this exception here. There is also Spanner, which is an exception to the exception and used its instance id before the upgrade. Given the limited number of Spanner customers and the fact that this additional exception only becomes relevant when adding a second source to a legacy pipeline, we could consider omitting this special case from the docs
| - Its data streams are still named `data:{rdi}:<schema_or_database>.<table>`, and its offset | ||
| and schema history keys are still `metadata:debezium:offsets` and | ||
| `metadata:debezium:schema_history`. | ||
| - Its `server_name` is still `rdi`. |
There was a problem hiding this comment.
... except if it's Spanner, then it would be the instance id.
| table: customers | ||
| ``` | ||
|
|
||
| When a pipeline has more than one source, every job must set `server_name`, and the value |
There was a problem hiding this comment.
| When a pipeline has more than one source, every job must set `server_name`, and the value | |
| When a pipeline has more than one source, every job (apart from the default job) must set `server_name`, and the value |
There was a problem hiding this comment.
Added the default job exception with somewhat different wording.
| pipeline status is not broken down per source. RDI reports the whole pipeline in an error state when a | ||
| single source fails, so you should use the `Components` section to find out which one has failed. | ||
|
|
||
| Each source's collector has its own metric collection, named after the collector, such as |
There was a problem hiding this comment.
We don't have collections for Spanner and Snowflake right now.
Also, the incoming entries and latency metrics described here are for the classic processor. Since the Flink processor becomes the default now, we should probably link to its metrics instead of the classic processor
See https://redislabs.atlassian.net/browse/RDSC-5610
Enhances #3841 with numerous changes and additions. I couldn't update the original PR since I don't have write permissions to this repository.
Note that it now documents 2 PRs that are not yet merged, but should be before the release:
Note
Low Risk
Documentation-only changes; operational risk is misconfiguration if published before the referenced RDI product changes ship.
Overview
Documents multi-source RDI pipelines: one pipeline can ingest from several source databases (each with its own collector, secrets, streams, and metrics), with a new Multiple sources in one pipeline guide covering naming,
config.yaml, jobs (server_name), add/remove sources, per-sourcestop/start/reset, monitoring, and legacy naming after upgrade.Secrets and deploy are rewritten around per-database keys (
USERNAME,PASSWORD, TLS keys) andredis-di set-secret … --db <source|target>, with env vars like${MYSQL_DB_PASSWORD}; K8s examples move fromsource-dbto<source>-db. LegacySOURCE_DB_*/TARGET_DB_*remain for single-source or pre-upgrade sources.Product defaults and fit: architecture, FAQ, pipeline config, and upgrade pages state
processors.typedefaults to Flink (setclassicexplicitly to opt out), including a 2.0.0 upgrade warning that undeployedtypepipelines move to Flink on next deploy. The “when to use” embed and decision tree drop the single-source-only gate and allow one or more sources.Cross-cutting updates cover API v2 requirement for multi-source, DLQ/stream naming with source segments, Flink job matcher lists/
regex:, observability per-collector URLs, HA/standby VM behavior, and refreshed database prep guides (MongoDB keystore, MySQL TLS, Snowflake/Spanner secrets, etc.).Reviewed by Cursor Bugbot for commit 4c412ec. Bugbot is set up for automated code reviews on this repo. Configure here.