-
Notifications
You must be signed in to change notification settings - Fork 34
[Schema Change] Add gRPC channel to notify node subscribe MQ #612
Conversation
Codecov Report
@@ Coverage Diff @@
## master #612 +/- ##
============================================
- Coverage 32.77% 31.96% -0.81%
Complexity 1248 1248
============================================
Files 522 525 +3
Lines 13111 13439 +328
Branches 1610 1657 +47
============================================
- Hits 4297 4296 -1
- Misses 8229 8558 +329
Partials 585 585
Continue to review full report at Codecov.
|
...ta_plane_manager/src/main/java/com/futurewei/alcor/dataplane/client/NodeSubscribeClient.java
Outdated
Show resolved
Hide resolved
| message NodeSubscribeInfo { | ||
| SubscribeOperation subscribe_operation = 1; | ||
| string topic = 2; | ||
| string key = 3; |
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.
What does the key differ from topic?
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.
The key is for key-shared mode in Pulsar. It's similar to the concept of a secondary topic in the message queue (e.g. key in Apache Pulsar, tag in RocketMQ). At the same time, this value can be set to null when it is not needed.
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.
It would be great to put the comment of "Allow to set NULL when a secondary topic is not needed" for the key field. @VanderChen
xieus
left a comment
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.
@VanderChen I have one last minor comment. Please take a look and if you can fix it in your other PR, that would be great.
No description provided.