You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@eventmesh.apache.org by "pandalee99 (via GitHub)" <gi...@apache.org> on 2023/09/25 11:26:37 UTC
[PR] [Enhancement] Do some code optimization.[StreamTopicConfig] (eventmesh)
pandalee99 opened a new pull request, #4462:
URL: https://github.com/apache/eventmesh/pull/4462
<!--
### Contribution Checklist
- Name the pull request in the form "[ISSUE #XXXX] Title of the pull request",
where *XXXX* should be replaced by the actual issue number.
Skip *[ISSUE #XXXX]* if there is no associated github issue for this pull request.
- Fill out the template below to describe the changes contributed by the pull request.
That will give reviewers the context they need to do the review.
- Each pull request should address only one issue.
Please do not mix up code from multiple issues.
- Each commit in the pull request should have a meaningful commit message.
- Once all items of the checklist are addressed, remove the above text and this checklist,
leaving only the filled out template below.
(The sections below can be removed for hotfixes of typos)
-->
<!--
(If this PR fixes a GitHub issue, please add `Fixes #<XXX>` or `Closes #<XXX>`.)
-->
Fixes #4192.
### Motivation
1. I deleted the transient
2. I created a method to get information on the specification
3. I optimized the code
### Modifications
Optimize this class to make the code more concise as needed
I try to submit the code and hope it doesn't go wrong
### Documentation
- Does this pull request introduce a new feature? (no)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
Re: [PR] [Enhancement] Do some code optimization.[StreamTopicConfig] (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #4462:
URL: https://github.com/apache/eventmesh/pull/4462#discussion_r1335791671
##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/core/protocol/grpc/consumer/consumergroup/StreamTopicConfig.java:
##########
@@ -40,19 +40,23 @@ public class StreamTopicConfig extends ConsumerGroupTopicConfig {
/**
* Key: IDC Value: list of emitters with Client_IP:port
*/
- private final transient Map<String, Map<String, EventEmitter<CloudEvent>>> idcEmitterMap = new ConcurrentHashMap<>();
+ private final Map<String, Map<String, EventEmitter<CloudEvent>>> idcEmitterMap = new ConcurrentHashMap<>();
/**
* Key: IDC Value: list of emitters
*/
- private transient Map<String, List<EventEmitter<CloudEvent>>> idcEmitters = new ConcurrentHashMap<>();
+ private Map<String, List<EventEmitter<CloudEvent>>> idcEmitters = new ConcurrentHashMap<>();
- private transient List<EventEmitter<CloudEvent>> totalEmitters = new ArrayList<>();
+ private List<EventEmitter<CloudEvent>> totalEmitters = new ArrayList<>();
public StreamTopicConfig(final String consumerGroup, final String topic, final SubscriptionMode subscriptionMode) {
super(consumerGroup, topic, subscriptionMode, GrpcType.STREAM);
}
+ public String createKey(String ip, String pid) {
+ return ip + ":" + pid;
+ }
+
Review Comment:
1. It will be better to mark this method with `private` modifier.
2. `concat()` is better than `+` operator here. The concat() method is more strict in what it accepts and has a better performance.
3. Maybe the `concatKey()` method name will be better, because the key has been provided instead of `Generated`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [Enhancement] Do some code optimization.[StreamTopicConfig] (eventmesh)
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #4462:
URL: https://github.com/apache/eventmesh/pull/4462#issuecomment-1733722331
## [Codecov](https://app.codecov.io/gh/apache/eventmesh/pull/4462?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#4462](https://app.codecov.io/gh/apache/eventmesh/pull/4462?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (6f60e37) into [master](https://app.codecov.io/gh/apache/eventmesh/commit/b4feffcf5c6dcb94cee447c4c7534985cfd1912c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (b4feffc) will **decrease** coverage by `0.02%`.
> Report is 1 commits behind head on master.
> The diff coverage is `0.00%`.
> :exclamation: Current head 6f60e37 differs from pull request most recent head f586736. Consider uploading reports for the commit f586736 to get more accurate results
```diff
@@ Coverage Diff @@
## master #4462 +/- ##
============================================
- Coverage 15.98% 15.97% -0.02%
Complexity 1475 1475
============================================
Files 690 690
Lines 27480 27475 -5
Branches 2567 2565 -2
============================================
- Hits 4392 4388 -4
+ Misses 22640 22637 -3
- Partials 448 450 +2
```
| [Files Changed](https://app.codecov.io/gh/apache/eventmesh/pull/4462?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...grpc/consumer/consumergroup/StreamTopicConfig.java](https://app.codecov.io/gh/apache/eventmesh/pull/4462?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZXZlbnRtZXNoLXJ1bnRpbWUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9ydW50aW1lL2NvcmUvcHJvdG9jb2wvZ3JwYy9jb25zdW1lci9jb25zdW1lcmdyb3VwL1N0cmVhbVRvcGljQ29uZmlnLmphdmE=) | `0.00% <0.00%> (ø)` | |
... and [7 files with indirect coverage changes](https://app.codecov.io/gh/apache/eventmesh/pull/4462/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [Enhancement] Do some code optimization.[StreamTopicConfig] (eventmesh)
Posted by "pandaapo (via GitHub)" <gi...@apache.org>.
pandaapo commented on code in PR #4462:
URL: https://github.com/apache/eventmesh/pull/4462#discussion_r1335892828
##########
eventmesh-runtime/src/test/java/org/apache/eventmesh/runtime/demo/CClientDemo.java:
##########
@@ -45,13 +42,10 @@ public static void main(String[] args) throws Exception {
client.justSubscribe(ASYNC_TOPIC, SubscriptionMode.CLUSTERING, SubscriptionType.ASYNC);
client.justSubscribe(BROADCAST_TOPIC, SubscriptionMode.BROADCASTING, SubscriptionType.ASYNC);
client.listen();
- client.registerSubBusiHandler(new ReceiveMsgHook() {
- @Override
- public void handle(Package msg, ChannelHandlerContext ctx) {
- if (msg.getHeader().getCmd() == Command.ASYNC_MESSAGE_TO_CLIENT || msg.getHeader().getCmd() == Command.BROADCAST_MESSAGE_TO_CLIENT) {
- if (log.isInfoEnabled()) {
- log.info("receive message: {}", msg);
- }
+ client.registerSubBusiHandler((msg, ctx) -> {
+ if (msg.getHeader().getCmd() == Command.ASYNC_MESSAGE_TO_CLIENT || msg.getHeader().getCmd() == Command.BROADCAST_MESSAGE_TO_CLIENT) {
+ if (log.isInfoEnabled()) {
+ log.info("receive message: {}", msg);
Review Comment:
These modifications has been merged in your PR #4461.
You should create a new branch to do your work. Otherwise, the merged code will be resubmitted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [Enhancement] Do some code optimization.[StreamTopicConfig] (eventmesh)
Posted by "pandalee99 (via GitHub)" <gi...@apache.org>.
pandalee99 commented on code in PR #4462:
URL: https://github.com/apache/eventmesh/pull/4462#discussion_r1336123682
##########
eventmesh-runtime/src/test/java/org/apache/eventmesh/runtime/demo/CClientDemo.java:
##########
@@ -45,13 +42,10 @@ public static void main(String[] args) throws Exception {
client.justSubscribe(ASYNC_TOPIC, SubscriptionMode.CLUSTERING, SubscriptionType.ASYNC);
client.justSubscribe(BROADCAST_TOPIC, SubscriptionMode.BROADCASTING, SubscriptionType.ASYNC);
client.listen();
- client.registerSubBusiHandler(new ReceiveMsgHook() {
- @Override
- public void handle(Package msg, ChannelHandlerContext ctx) {
- if (msg.getHeader().getCmd() == Command.ASYNC_MESSAGE_TO_CLIENT || msg.getHeader().getCmd() == Command.BROADCAST_MESSAGE_TO_CLIENT) {
- if (log.isInfoEnabled()) {
- log.info("receive message: {}", msg);
- }
+ client.registerSubBusiHandler((msg, ctx) -> {
+ if (msg.getHeader().getCmd() == Command.ASYNC_MESSAGE_TO_CLIENT || msg.getHeader().getCmd() == Command.BROADCAST_MESSAGE_TO_CLIENT) {
+ if (log.isInfoEnabled()) {
+ log.info("receive message: {}", msg);
Review Comment:
I see. Thanks for the guidance!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [Enhancement] Do some code optimization.[StreamTopicConfig] (eventmesh)
Posted by "pandaapo (via GitHub)" <gi...@apache.org>.
pandaapo commented on PR #4462:
URL: https://github.com/apache/eventmesh/pull/4462#issuecomment-1734641306
Need a new branch for this issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [Enhancement] Do some code optimization.[StreamTopicConfig] (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #4462:
URL: https://github.com/apache/eventmesh/pull/4462#discussion_r1336024654
##########
eventmesh-runtime/src/test/java/org/apache/eventmesh/runtime/demo/CClientDemo.java:
##########
@@ -45,13 +42,10 @@ public static void main(String[] args) throws Exception {
client.justSubscribe(ASYNC_TOPIC, SubscriptionMode.CLUSTERING, SubscriptionType.ASYNC);
client.justSubscribe(BROADCAST_TOPIC, SubscriptionMode.BROADCASTING, SubscriptionType.ASYNC);
client.listen();
- client.registerSubBusiHandler(new ReceiveMsgHook() {
- @Override
- public void handle(Package msg, ChannelHandlerContext ctx) {
- if (msg.getHeader().getCmd() == Command.ASYNC_MESSAGE_TO_CLIENT || msg.getHeader().getCmd() == Command.BROADCAST_MESSAGE_TO_CLIENT) {
- if (log.isInfoEnabled()) {
- log.info("receive message: {}", msg);
- }
+ client.registerSubBusiHandler((msg, ctx) -> {
+ if (msg.getHeader().getCmd() == Command.ASYNC_MESSAGE_TO_CLIENT || msg.getHeader().getCmd() == Command.BROADCAST_MESSAGE_TO_CLIENT) {
+ if (log.isInfoEnabled()) {
+ log.info("receive message: {}", msg);
Review Comment:
Yes, a new branch is needed to resolve another issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [Enhancement] Do some code optimization.[StreamTopicConfig] (eventmesh)
Posted by "pandaapo (via GitHub)" <gi...@apache.org>.
pandaapo closed pull request #4462: [Enhancement] Do some code optimization.[StreamTopicConfig]
URL: https://github.com/apache/eventmesh/pull/4462
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
Re: [PR] [Enhancement] Do some code optimization.[StreamTopicConfig] (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #4462:
URL: https://github.com/apache/eventmesh/pull/4462#discussion_r1336024654
##########
eventmesh-runtime/src/test/java/org/apache/eventmesh/runtime/demo/CClientDemo.java:
##########
@@ -45,13 +42,10 @@ public static void main(String[] args) throws Exception {
client.justSubscribe(ASYNC_TOPIC, SubscriptionMode.CLUSTERING, SubscriptionType.ASYNC);
client.justSubscribe(BROADCAST_TOPIC, SubscriptionMode.BROADCASTING, SubscriptionType.ASYNC);
client.listen();
- client.registerSubBusiHandler(new ReceiveMsgHook() {
- @Override
- public void handle(Package msg, ChannelHandlerContext ctx) {
- if (msg.getHeader().getCmd() == Command.ASYNC_MESSAGE_TO_CLIENT || msg.getHeader().getCmd() == Command.BROADCAST_MESSAGE_TO_CLIENT) {
- if (log.isInfoEnabled()) {
- log.info("receive message: {}", msg);
- }
+ client.registerSubBusiHandler((msg, ctx) -> {
+ if (msg.getHeader().getCmd() == Command.ASYNC_MESSAGE_TO_CLIENT || msg.getHeader().getCmd() == Command.BROADCAST_MESSAGE_TO_CLIENT) {
+ if (log.isInfoEnabled()) {
+ log.info("receive message: {}", msg);
Review Comment:
Yes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [Enhancement] Do some code optimization.[StreamTopicConfig] (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #4462:
URL: https://github.com/apache/eventmesh/pull/4462#discussion_r1335791671
##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/core/protocol/grpc/consumer/consumergroup/StreamTopicConfig.java:
##########
@@ -40,19 +40,23 @@ public class StreamTopicConfig extends ConsumerGroupTopicConfig {
/**
* Key: IDC Value: list of emitters with Client_IP:port
*/
- private final transient Map<String, Map<String, EventEmitter<CloudEvent>>> idcEmitterMap = new ConcurrentHashMap<>();
+ private final Map<String, Map<String, EventEmitter<CloudEvent>>> idcEmitterMap = new ConcurrentHashMap<>();
/**
* Key: IDC Value: list of emitters
*/
- private transient Map<String, List<EventEmitter<CloudEvent>>> idcEmitters = new ConcurrentHashMap<>();
+ private Map<String, List<EventEmitter<CloudEvent>>> idcEmitters = new ConcurrentHashMap<>();
- private transient List<EventEmitter<CloudEvent>> totalEmitters = new ArrayList<>();
+ private List<EventEmitter<CloudEvent>> totalEmitters = new ArrayList<>();
public StreamTopicConfig(final String consumerGroup, final String topic, final SubscriptionMode subscriptionMode) {
super(consumerGroup, topic, subscriptionMode, GrpcType.STREAM);
}
+ public String createKey(String ip, String pid) {
+ return ip + ":" + pid;
+ }
+
Review Comment:
1. It will be better to mark this method with `private` modifier.
2. `concat()` is better than `+` operator here. The concat() method is more strict in what it accepts and has a better performance.
3. Maybe the `concatKey()` method name will be better, because the key has been provided already instead of `Generated`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [Enhancement] Do some code optimization.[StreamTopicConfig] (eventmesh)
Posted by "pandalee99 (via GitHub)" <gi...@apache.org>.
pandalee99 commented on code in PR #4462:
URL: https://github.com/apache/eventmesh/pull/4462#discussion_r1335804744
##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/core/protocol/grpc/consumer/consumergroup/StreamTopicConfig.java:
##########
@@ -40,19 +40,23 @@ public class StreamTopicConfig extends ConsumerGroupTopicConfig {
/**
* Key: IDC Value: list of emitters with Client_IP:port
*/
- private final transient Map<String, Map<String, EventEmitter<CloudEvent>>> idcEmitterMap = new ConcurrentHashMap<>();
+ private final Map<String, Map<String, EventEmitter<CloudEvent>>> idcEmitterMap = new ConcurrentHashMap<>();
/**
* Key: IDC Value: list of emitters
*/
- private transient Map<String, List<EventEmitter<CloudEvent>>> idcEmitters = new ConcurrentHashMap<>();
+ private Map<String, List<EventEmitter<CloudEvent>>> idcEmitters = new ConcurrentHashMap<>();
- private transient List<EventEmitter<CloudEvent>> totalEmitters = new ArrayList<>();
+ private List<EventEmitter<CloudEvent>> totalEmitters = new ArrayList<>();
public StreamTopicConfig(final String consumerGroup, final String topic, final SubscriptionMode subscriptionMode) {
super(consumerGroup, topic, subscriptionMode, GrpcType.STREAM);
}
+ public String createKey(String ip, String pid) {
+ return ip + ":" + pid;
+ }
+
Review Comment:
thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org