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.

Sign up for GitHub

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

Jump to bottom

[ISSUE #70] Retained message based on raft state machine #131

Merged
merged 25 commits into from
Oct 21, 2022
Merged

[ISSUE #70] Retained message based on raft state machine #131

merged 25 commits into from
Oct 21, 2022

Conversation

YYYYWD
Copy link
Contributor

@YYYYWD YYYYWD commented Aug 1, 2022

No description provided.

@YYYYWD
Copy link
Contributor Author

YYYYWD commented Aug 1, 2022

#70

@ShannonDing ShannonDing changed the title Retained message based on raft state machine [ISSUE #70] Retained message based on raft state machine Aug 1, 2022
@ShannonDing ShannonDing requested a review from pingww August 1, 2022 07:35
@ShannonDing
Copy link
Member

link #118

@ShannonDing ShannonDing added the enhancement New feature or request label Aug 1, 2022
@ShannonDing
Copy link
Member

the GID separator has been changed from "=" to "%",
rebase the latest develop branch and merge the PR #129 .

ShannonDing
ShannonDing requested changes Aug 1, 2022
View reviewed changes
Copy link
Member

@ShannonDing ShannonDing left a comment

Choose a reason for hiding this comment

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

the GID separator has been changed from "=" to "%",
rebase the latest develop branch and merge the PR #129 .

@ShannonDing ShannonDing mentioned this pull request Aug 1, 2022
Closed
@YYYYWD
Copy link
Contributor Author

YYYYWD commented Aug 3, 2022

the GID separator has been changed from "=" to "%", rebase the latest develop branch and merge the PR #129 .

Using % can cause unpredictable errors, so use "-" instead

@DongyuanPan
Copy link
Contributor

@YYYYWD Y Using % can cause unpredictable errors, so use "-" instead. it can make a new pr to develop

@DongyuanPan
Copy link
Contributor

Does the state machine need to care about message content? Or use byte array storage directly, reducing serialization and deserialization. Trie tree retainedMsgTopicTrie in state machine also has the same problem

@DongyuanPan
Copy link
Contributor

To facilitate troubleshooting, you need to add error logs

@codecov-commenter
Copy link

codecov-commenter commented Aug 8, 2022 •
edited
Loading

Codecov Report

Merging #131 (ca54807) into develop (af04be9) will decrease coverage by 1.93%.
The diff coverage is 17.79%.

@@             Coverage Diff             @@
##           develop     #131      +/-   ##
===========================================
- Coverage    41.96%   40.02%   -1.94%     
===========================================
  Files          119      122       +3     
  Lines         4904     5314     +410     
  Branches       714      764      +50     
===========================================
+ Hits          2058     2127      +69     
- Misses        2491     2824     +333     
- Partials       355      363       +8
Impacted Files Coverage Δ
...org/apache/rocketmq/mqtt/meta/config/MetaConf.java 0.00% <0.00%> (ø)
...apache/rocketmq/mqtt/meta/raft/MqttRaftServer.java 0.00% <0.00%> (ø)
...ache/rocketmq/mqtt/meta/raft/MqttStateMachine.java 0.00% <0.00%> (ø)
...e/rocketmq/mqtt/meta/raft/processor/Constants.java 0.00% <ø> (ø)
...qtt/meta/raft/processor/CounterStateProcessor.java 0.00% <0.00%> (ø)
...meta/raft/processor/RetainedMsgStateProcessor.java 0.00% <0.00%> (ø)
...ketmq/mqtt/meta/raft/processor/StateProcessor.java 0.00% <ø> (ø)
...ocketmq/mqtt/common/hook/AbstractUpstreamHook.java 21.05% <ø> (ø)
...g/apache/rocketmq/mqtt/common/model/Constants.java 0.00% <ø> (ø)
...pache/rocketmq/mqtt/common/model/Subscription.java 77.14% <0.00%> (-4.68%) ⬇️
... and 16 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

DongyuanPan
DongyuanPan reviewed Aug 8, 2022
View reviewed changes
conf/meta.conf Outdated Show resolved Hide resolved
@DongyuanPan
Copy link
Contributor

Please adjust the code style, remove extra blank lines, and add spaces around certain symbols

DongyuanPan
DongyuanPan approved these changes Aug 30, 2022
View reviewed changes
Copy link
Contributor

@DongyuanPan DongyuanPan left a comment

Choose a reason for hiding this comment

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

GOOD JOB

@DongyuanPan
Copy link
Contributor

@ShannonDing @pingww it can be merged

@pingww
Copy link
Contributor

pingww commented Oct 7, 2022

to resolve conflicts

@pingww pingww self-requested a review October 20, 2022 06:30
@pingww pingww merged commit 748c841 into apache:develop Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShannonDing ShannonDing ShannonDing requested changes

@DongyuanPan DongyuanPan DongyuanPan approved these changes

@pingww pingww Awaiting requested review from pingww

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@YYYYWD @ShannonDing @DongyuanPan @codecov-commenter @pingww