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.

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

Added shared subscription support preparation code. #752

Merged
merged 3 commits into from
Dec 9, 2020

Conversation

redboltz
Copy link
Owner

@redboltz redboltz commented Dec 8, 2020

Broker module files are separated by responsibility.
Fixed invalid memory access on endpoint_t.

e.con.reset();
if (e.session_expiry_interval && e.session_expiry_interval.value() != std::chrono::seconds(session_never_expire)) {
e.tim_session_expiry = std::make_shared<as::steady_timer>(ioc_, e.session_expiry_interval.value());
e.tim_session_expiry->async_wait(
[this, wp = std::weak_ptr<as::steady_timer>(e.tim_session_expiry)](error_code ec) {
if (auto sp = wp.lock()) {
if (!ec) {
sessions_.get<tag_tim>().erase(sp);
}
}
}
);
}
// TopicAlias lifetime is the same as Session lifetime
// It is different from MQTT v5 spec but practical choice.
// See
// https://lists.oasis-open.org/archives/mqtt-comment/202009/msg00000.html
e.topic_alias_recv = ep.get_topic_alias_recv_container();

@redboltz redboltz force-pushed the add_shared_subscriptions_prepare branch 8 times, most recently from 65f2784 to b2b5767 Compare December 8, 2020 08:44
@codecov
Copy link

codecov bot commented Dec 8, 2020

Codecov Report

Merging #752 (1aeb108) into master (45af140) will decrease coverage by 0.07%.
The diff coverage is 88.91%.

@@            Coverage Diff             @@
##           master     #752      +/-   ##
==========================================
- Coverage   85.59%   85.52%   -0.08%     
==========================================
  Files          50       60      +10     
  Lines        8363     8557     +194     
==========================================
+ Hits         7158     7318     +160     
- Misses       1205     1239      +34     

@redboltz redboltz force-pushed the add_shared_subscriptions_prepare branch from b2b5767 to 33bdd9f Compare December 8, 2020 12:35
@redboltz
Copy link
Owner Author

redboltz commented Dec 8, 2020

I implemented share name based shared subscriptions.

s1: $share/sn1/t1
s1: $share/sn1/t2
s2: $share/sn1/t1
s3: $share/sn1/t1
s3: $share/sn1/t2

A publisher publishes t1,t2,t1,t2.... continuously.

client share name topic expected delivery order
s1 sn1 t1 1,7,9
s1 sn1 t2 4
s2 sn1 t2 2,6,10
s3 sn1 t1 3,5,11
s3 sn1 t2 8

I wrote test the step1 to 8, then it works as I expected.

I think that this PR is good start point.
After I merged the PR, I will add strategy switching mechanism.

The concept is quite simple.

class shared_target_concept {
public:
    void insert(buffer share_name, buffer topic_filter, session_state& ss);
    void erase(buffer share_name, buffer topic_filter, session_state const& ss);
    void erase(session_state const& ss);
    optional<session_state_ref> get_target(buffer const& share_name, buffer const& topic_filter);
};

The class that implements custom strategy need to implement above four member functions.
I don't decide yet how to implement the concept switcher. e.g. Template base, virtual function base, others...

In addition, I noticed that there are some protocol errors that are related to shared subscriptions.
For example, the combination of NL bit 1 and share name is protocol error.
In this PR, parsing share name is at the broker side, but perhaps it should be endpoint.hpp (process_subscribe).

NOTE: endpoint.hpp would be separated by responsibility like as broker in the PR, but it is a different topic.

Long thread id was mapped to the index.
But it was thread unsafe even if logger itself is thread safe.
So I removed the feature.
Now, default logging setup outputs raw thread id.
@redboltz redboltz force-pushed the add_shared_subscriptions_prepare branch from 2dd837c to 1aeb108 Compare December 9, 2020 03:04
@redboltz redboltz merged commit 5c2052b into master Dec 9, 2020
@redboltz redboltz deleted the add_shared_subscriptions_prepare branch December 9, 2020 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant