Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ticdc: update pulsar sink #4294

Merged
merged 5 commits into from
Aug 27, 2020
Merged

ticdc: update pulsar sink #4294

merged 5 commits into from
Aug 27, 2020

Conversation

overvenus
Copy link
Member

@overvenus overvenus commented Aug 19, 2020

What is changed, added or deleted? (Required)

Update the Apache Pulsar sink configuration.

Which TiDB version(s) do your changes apply to? (Required)

  • master (the latest development version)
  • v4.0 (TiDB 4.0 versions)
  • v3.1 (TiDB 3.1 versions)
  • v3.0 (TiDB 3.0 versions)
  • v2.1 (TiDB 2.1 versions)

What is the related PR or file link(s)?

Do your changes match any of the following descriptions?

  • Delete files
  • Change aliases
  • Have version specific changes
  • Might cause conflicts

Signed-off-by: Neil Shen <overvenus@gmail.com>
Signed-off-by: Neil Shen <overvenus@gmail.com>
Signed-off-by: Neil Shen <overvenus@gmail.com>
@overvenus
Copy link
Member Author

PTAL @TomShawn @amyangfei

@TomShawn TomShawn requested review from TomShawn and amyangfei August 20, 2020 10:06
@TomShawn TomShawn added needs-cherry-pick-4.0 size/medium Changes of a medium size. status/PTAL This PR is ready for reviewing. translation/doing This PR’s assignee is translating this PR. labels Aug 20, 2020
@ti-srebot
Copy link
Contributor

@amyangfei, @TomShawn, PTAL.

@@ -111,64 +111,98 @@ Info: {"sink-uri":"mysql://root:123456@127.0.0.1:3306/","opts":{},"create-time":
```

- `--changefeed-id`: 同步任务的 ID,格式需要符合正则表达式 `^[a-zA-Z0-9]+(\-[a-zA-Z0-9]+)*$`。如果不指定该 ID,TiCDC 会自动生成一个 UUID(version 4 格式)作为 ID。
- `--sink-uri`: 同步任务下游的地址,需要按照以下格式进行配置,目前 scheme 支持 `mysql`/`tidb`/`kafka`。
- `--sink-uri`: 同步任务下游的地址,需要按照以下格式进行配置,目前 scheme 支持 `mysql`/`tidb`/`kafka`/`pulsar`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mysql/tidb/pulsar 同样支持 mysql+ssl/tidb+ssl/pulsar+ssl,是否需要也在文档里写出

Copy link
Member Author

@overvenus overvenus Aug 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我觉得没必要,因为 SSL/TLS 开启与否在于 URI 中的参数,而不是 scheme。

Copy link
Contributor

@amyangfei amyangfei Aug 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

那代码里 +ssl 的存在也没有意义了,现阶段 scheme 中的 +ssl 在代码里只是标识,不做检查

@TomShawn TomShawn added status/require-change Needs the author to address comments. and removed status/PTAL This PR is ready for reviewing. labels Aug 24, 2020
@ti-srebot
Copy link
Contributor

@overvenus, please update your pull request.

Copy link
Contributor

@amyangfei amyangfei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot
Copy link
Contributor

@amyangfei,Thanks for your review. However, LGTM is restricted to Reviewers or higher roles.See the corresponding SIG page for more information. Related SIGs: docs(slack).

@TomShawn TomShawn assigned TomShawn and unassigned Joyinqin Aug 27, 2020
Copy link
Contributor

@zier-one zier-one left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot
Copy link
Contributor

@leoppro,Thanks for your review. However, LGTM is restricted to Reviewers or higher roles.See the corresponding SIG page for more information. Related SIGs: docs(slack).

Copy link
Contributor

@TomShawn TomShawn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 27, 2020
@TomShawn TomShawn merged commit c932abe into pingcap:master Aug 27, 2020
ti-srebot pushed a commit to ti-srebot/docs-cn that referenced this pull request Aug 27, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Aug 27, 2020
9 tasks
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #4359

ti-srebot added a commit that referenced this pull request Aug 27, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

Co-authored-by: Neil Shen <overvenus@gmail.com>
@overvenus overvenus deleted the cdc/pulsar branch August 27, 2020 08:31
@TomShawn TomShawn added translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR. and removed translation/doing This PR’s assignee is translating this PR. labels Aug 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/medium Changes of a medium size. status/LGT1 Indicates that a PR has LGTM 1. status/require-change Needs the author to address comments. translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants