Skip to content

[Feature #4793] Support MQTT protocol #4794

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

Open
wants to merge 4,467 commits into
base: master
Choose a base branch
from
Open

Conversation

karsonto
Copy link
Member

Fixes #4793

Motivation

Explain the content here.
Explain why you want to make the changes and what problem you're trying to solve.

Modifications

Describe the modifications you've done.

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

qqeasonchen and others added 30 commits August 1, 2023 11:16
…eadUtils] (apache#4297)

* Update ThreadUtils.java

* Update ThreadUtils.java
…ectors] (apache#4314)

* [ISSUE apache#4266]: Retore interrupted state for interrupted exception in open function sinc connector

* [ISSUE apache#4266]: Retore interrupted state for interrupted exception in open function source connector

* [ISSUE apache#4266]: Retore interrupted state for interrupted exception in RocketMQSink connector
…pache#4315)

* [ISSUE apache#4093]: Refactor package building chunks to reusable method

* [ISSUE apache#4093]: Refactor user agent building chunks to reusable method
…aced with lambda[HelloTask] (apache#4316)

* [4260] Fixed: Anonymous new ChannelFutureListener() can be replaced with lambda[HelloTask]

* Upgraded: First Interaction Action Version to Latest

* [4260] Removed: Unused Import

* Update package import order in HelloTask.java

---------

Co-authored-by: mike_xwm <[email protected]>
* fix: add cloudEventCodec for redis connector.

* fix: add cloudEventCodec for redis connector.

* fix import order.

* fix: add doc.

* fix: adjust cloudevent encode and decode.
…che#4337)

* issue-4262 Enhancement Request EventMeshCloudEventUtils

* issue-4262 Enhancement Request EventMeshCloudEventUtils move on top method getValue

* issue-4262 Enhancement Request EventMeshCloudEventUtils remove redundant else for Optional.of

---------

Co-authored-by: maxim.zgardan <[email protected]>
[ISSUE apache#4328] Add offsetManagement Service for connectors
[ISSUE apache#4268] Used switch to replace the if-else [CloudEventsProtocolAdaptor]
… test coverage rate (apache#4340)

* issues apache#4264 commit

* unit

* issues apache#4339 unit test coverage

* issues apache#4339 WebhookProtocolTransportObjectTest fix

* issues apache#4339 WebhookProtocolTransportObjectTest fix

* issues apache#4339 WebhookProtocolTransportObjectTest fix

* issues apache#4339 WebhookProtocolTransportObjectTest fix

* issues apache#4339 check style

* issues apache#4339 new instance replace builder.

* issues apache#4339 WebhookProtocolTransportObjectTest unit test.

* issues apache#4339 check style.

* issues apache#4339 check style.

* issues apache#4339 check style.

* fixed.

* fixed.

* fixed.

* fixed.

* fixed.

* delete java bean unit test.

* fix build error.
* fix start error and some code optimization.

* fix code style
…ache#4348)

* [ISSUE apache#4339][Unit Test] eventmesh-common header unit test.

* [ISSUE apache#4339][Unit Test] eventmesh-common header unit test.

* [ISSUE apache#4339][Unit Test] eventmesh-common body unit test.

* [ISSUE apache#4339][Unit Test] eventmesh-common ReflectUtils and RandomStringUtils unit test.

* [ISSUE apache#4339]comment.

* [ISSUE apache#4339]expectedException.
…hrow java.lang.NullPointerException (apache#4352)

* [ISSUE apache#4345]Fix publish EventMeshMessage without requestCode throw java.lang.NullPointerException

* optimize logic
orol116 and others added 12 commits February 1, 2024 23:43
…y server (apache#4739)

* fix 4738

* fix some bug

* fix bug

* remove initProducerManager from AbstractRemotingServer init

* bug fix

* bug fix

* some enhance

* some enhance

* add admin bootstrap

* some enhance

* remove HttpHandlerManager and ClientManageController.

* modify some unit test

* add admin http handlermanager
* [ISSUE apache#4458] Support mysql Sink Connector feature

* remove pg jdbc import

* update dependencies
…ache#4777)

* Add null check in writeOffset method

* delete todo

* Move data.put inside null check in writeOffset method

* simplify if judgement

* remove dev environment

* fix style
a. Change to private modifier.
b. Repeat code extraction as method.
* condition_check_for_source_eorker

* ci_typo_fix

* import_order_fix
Copy link

codecov bot commented Mar 19, 2024

Codecov Report

Attention: Patch coverage is 38.42207% with 960 lines in your changes missing coverage. Please review.

Project coverage is 16.11%. Comparing base (1600c6b) to head (44e61e9).
Report is 4687 commits behind head on master.

Files with missing lines Patch % Lines
...mesh/connector/jdbc/connection/JdbcConnection.java 0.00% 137 Missing ⚠️
...nnector/file/sink/connector/FileSinkConnector.java 0.00% 62 Missing ⚠️
...r/jdbc/dialect/AbstractGeneralDatabaseDialect.java 0.00% 56 Missing ⚠️
...g/apache/eventmesh/common/utils/JsonPathUtils.java 0.00% 54 Missing ⚠️
...ava/org/apache/eventmesh/common/utils/LogUtil.java 7.54% 48 Missing and 1 partial ⚠️
...tor/jdbc/connection/mysql/MysqlJdbcConnection.java 0.00% 43 Missing ⚠️
...a/org/apache/eventmesh/connector/jdbc/Payload.java 0.00% 32 Missing ⚠️
...dingtalk/sink/connector/DingDingSinkConnector.java 65.38% 24 Missing and 3 partials ⚠️
...pache/eventmesh/connector/jdbc/CatalogChanges.java 0.00% 26 Missing ⚠️
...ava/org/apache/eventmesh/connector/jdbc/Field.java 0.00% 24 Missing ⚠️
... and 82 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #4794      +/-   ##
============================================
- Coverage     16.93%   16.11%   -0.82%     
- Complexity     1413     1734     +321     
============================================
  Files           589      870     +281     
  Lines         25786    31682    +5896     
  Branches       2397     2739     +342     
============================================
+ Hits           4367     5106     +739     
- Misses        20983    26097    +5114     
- Partials        436      479      +43     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.



protected final transient Map<MqttMessageType, MqttProcessor> processorTable =
new ConcurrentHashMap<>(64);
Copy link
Member

Choose a reason for hiding this comment

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

The class is not serializable, is the transient keyword redundant?

Copy link
Member Author

Choose a reason for hiding this comment

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

Noted,please review again.

} catch (Exception ex) {
log.error("EventMeshMQTTServer RemotingServer shutdown Err!", ex);
}
System.exit(-1);
Copy link
Member

Choose a reason for hiding this comment

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

Is it appropriate to exit the process when the MQTT server fails to start?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a reference to other EM protocols.If you want to modify it after the discussion, I will ignore this startup failure.

Copy link
Member

Choose a reason for hiding this comment

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

@xwm1992

Currently tcp protocol, http protocol server must be started successfully, grpc protocol did not so. Now added MQTT protocol server must be started successfullyt, please community to give advice, I can not decide.

目前tcp协议、http协议的server必须成功启动,grpc协议没有如此,现在新增MQTT协议的server是否必须启动成功,请社区给出意见,我权衡不好。

Copy link
Contributor

Choose a reason for hiding this comment

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

我理解这里的必须启动成功实际上是受是否开启MQTT协议的配置控制的吧?这里的退出我认为没有问题,如果MQTT协议加载有问题退出了,那其实可以在配置中移除MQTT协议,保证TCP、HTTP等协议正常启动服务就好。

this.cleanThread = new Thread(() -> {
while (true && !Thread.currentThread().isInterrupted()) {
try {
ClientInfo clientInfo = delayQueue.take();
Copy link
Member

Choose a reason for hiding this comment

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

What is the role of delayQueue?

Copy link
Member Author

Choose a reason for hiding this comment

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

The role of delayQueue is to clean up the time out connection.

Copy link
Member

Choose a reason for hiding this comment

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

Is it up to the client to decide and set the length of time that each client's connection survives when it sends a message?

每个客户端的连接的存活时长,是靠客户端发送消息时自己决定并设置好吗?

Copy link
Member Author

Choose a reason for hiding this comment

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

客户端可以携带keep alive time参数,用做会话存活时间


@Override
public void process(ChannelHandlerContext ctx, MqttMessage mqttMessage) throws MqttException {
//
Copy link
Member

Choose a reason for hiding this comment

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

Remove this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Noted,please review again.

protected final Map<MqttMessageType, MqttProcessor> processorTable =
new ConcurrentHashMap<>(64);

private final transient AtomicBoolean started = new AtomicBoolean(false);
Copy link
Member

Choose a reason for hiding this comment

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

Same problem.
I'm not familiar with MQTT, so the rest of review work needs the community to complete.

Copy link
Member

Choose a reason for hiding this comment

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

Same problem.

Waiting for your response.

Copy link
Member Author

Choose a reason for hiding this comment

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

what's the problem here?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Noted,please review again.

@xwm1992
Copy link
Contributor

xwm1992 commented Mar 26, 2024

@karsonto
I don’t understand the implementation of the publish processor, and I don’t see how messages of the mqtt protocol are converted and sent to eventmesh storage.


我没有理解对于publish processor的实现,没有看到将mqtt协议的消息进行转换后发送到eventmesh storage。

@karsonto
Copy link
Member Author

karsonto commented Mar 26, 2024

@xwm1992
This PR is the framework design of the MQTT protocol , and will continue to optimize persist messages in another PR.

@karsonto
Copy link
Member Author

This PR is the framework design of the MQTT protocol , and will continue to optimize persist messages in another PR.

Copy link
Member

@Pil0tXia Pil0tXia left a comment

Choose a reason for hiding this comment

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

Is eventmesh-protocol-plugin implementation the only work left for this protocol in Runtime?

@karsonto
Copy link
Member Author

karsonto commented Apr 15, 2024

Is eventmesh-protocol-plugin implementation the only work left for this protocol in Runtime?

I think there is unnecessry to implement this module for this time. If it needs to be implemented in the future, I will submit a PR to supplement it.

@Pil0tXia Pil0tXia added the need review The review work of this PR needs another reviewer/PMC's help label Jun 12, 2024
Copy link
Contributor

It has been 60 days since the last activity on this pull request. I am reaching out here to gently remind you that the Apache EventMesh community values every pull request, and please feel free to get in touch with the reviewers at any time. They are available to assist you in advancing the progress of your pull request and offering the latest feedback.

If you encounter any challenges during development, seeking support within the community is encouraged. We sincerely appreciate your contributions to Apache EventMesh.

Copy link
Contributor

It has been 60 days since the last activity on this pull request. I am reaching out here to gently remind you that the Apache EventMesh community values every pull request, and please feel free to get in touch with the reviewers at any time. They are available to assist you in advancing the progress of your pull request and offering the latest feedback.

If you encounter any challenges during development, seeking support within the community is encouraged. We sincerely appreciate your contributions to Apache EventMesh.

@github-actions github-actions bot added Stale and removed Stale labels Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need review The review work of this PR needs another reviewer/PMC's help
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Support MQTT protocol