-
Notifications
You must be signed in to change notification settings - Fork 283
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
sink: Pass extra meta about the event to MQ producer #1442
Conversation
Signed-off-by: Xiaoguang Sun <sunxiaoguang@zhihu.com>
Signed-off-by: Xiaoguang Sun <sunxiaoguang@zhihu.com>
4518985
to
518977f
Compare
Signed-off-by: Xiaoguang Sun <sunxiaoguang@zhihu.com>
Signed-off-by: Xiaoguang Sun <sunxiaoguang@zhihu.com>
/run-all-tests |
2 similar comments
/run-all-tests |
/run-all-tests |
/run-all-tests |
/run-all-tests |
Codecov Report
@@ Coverage Diff @@
## master #1442 +/- ##
================================================
- Coverage 47.8373% 47.8275% -0.0098%
================================================
Files 145 146 +1
Lines 14727 14845 +118
================================================
+ Hits 7045 7100 +55
- Misses 6946 7009 +63
Partials 736 736 |
Signed-off-by: Xiaoguang Sun <sunxiaoguang@zhihu.com>
94f52c9
to
25e46a1
Compare
/run-all-tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
cdc/sink/codec/interface.go
Outdated
ms := int64(m.Ts >> physicalShiftBits) | ||
return time.Unix(ms/1e3, (ms%1e3)*1e6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use github.com/pingcap/tidb/store/tikv/oracle.GetTimeFromTS
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, It sounds reasonable to reuse code from TiDB.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
cdc/sink/codec/interface.go
Outdated
ms := int64(m.Ts >> physicalShiftBits) | ||
return time.Unix(ms/1e3, (ms%1e3)*1e6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be simplified with oracle.GetTimeFromTS(m.Ts)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, It sounds reasonable to reuse code from TiDB.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
cdc/sink/codec/interface_test.go
Outdated
@@ -0,0 +1,121 @@ | |||
// Copyright 2020 PingCAP, Inc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Copyright 2020 PingCAP, Inc. | |
// Copyright 2021 PingCAP, Inc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Signed-off-by: Xiaoguang Sun <sunxiaoguang@zhihu.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/merge |
Your auto merge job has been accepted, waiting for:
|
Signed-off-by: Xiaoguang Sun <sunxiaoguang@zhihu.com>
/run-all-tests |
/merge |
Your auto merge job has been accepted, waiting for:
|
@sunxiaoguang merge failed. |
/run-all-tests |
/merge |
Your auto merge job has been accepted, waiting for:
|
/merge |
Your auto merge job has been accepted, waiting for:
|
/run-all-tests |
1 similar comment
/run-all-tests |
@sunxiaoguang merge failed. |
/run-all-tests |
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
cherry pick to release-4.0 in PR #1466 |
Pass extra meta about the event to mq producer
What problem does this PR solve?
Pass extra structured meta data (Ts, Schema, Table) for event in Context passed to MQ producer. We can use these extra meta data in message for application to seek quickly from messages in the past.
What is changed and how it works?
Put meta data about events in MQMessage and change MQ Producer interface to accept message instead of key and value slices.
Check List
Tests
Code changes
Has exported variable/fields change
Add Schema, Table, Type and Protocol fields to MQMessage
Has interface methods change
Change Producer to accept MQMessage instead of key and value slices
Side effects
Related changes
Release note