changing `subscriptionsByTopic`?

[behrad]

according to this discussion I'm trying to store only clientId in Qlobber instead of {clientId, qos, topic} object. The only barrier here is persistence subscriptionsByTopic which is returning those objects in enqueueOffline, however only a list of clientIds would suffice. I trace down for any access to that object, and here it is, only the removeSharp filter! What does that do? can we remove that check? or can we run that check against packet.topic?

This is the only blocker to switch Qlobber to clientId values @mcollina

[mcollina]

The removeSharp functionality must stay in, as the $SYS  topic is not matched by #. It can be moved somewhere else.

I think this change is more articulated than what you describe, because the same object goes into https://github.com/mcollina/aedes-persistence/blob/master/persistence.js#L170. It does not use the topic and QoS either.

[behrad]

It does not use the topic and QoS either.

Yes, we can change outgoingEnqueue spec for the first argument.

It can be moved somewhere else

I'm not sure where we can move it @mcollina

[mcollina]

I'm not sure where we can move it

I tend to agree, but if we can't move it we can't make this change. If you are subscribing with #, $SYS topics should not be enqueued.

[behrad]

@mcollina let me ask: Do $SYS topics ever need to be enqueued?

If not, why not to check packets topic is a SYS instead of checking on Qlobber's client topic.

[mcollina]

Do $SYS topics ever need to be enqueued?

I assume so. Mosquitto enqueues them.

[behrad]

I believe we can think to drop support for qos>0 subscriptions on $SYS topics.

A stateful $SYS subscriber means not a good idea to me... Broker should not be responsible for offline storage of $SYS messages. they can be huge also.

[mcollina]

I think that is needed to support the mosquitto bridge protocol.

Another idea is to block offline storage of messages from a # subscription, so a full # subscription can only do QoS 0. I think that's more reasonable, even if we want to break from the MQTT spec.

[behrad]

Another idea is to block offline storage of messages from a # subscription, so a full # subscription can only do QoS 0. I think that's more reasonable, even if we want to break from the MQTT spec.

That would need a change to handleSubscribe, yes?

[mcollina]

That would need a change to handleSubscribe, yes?

Yes.

cc @GavinDmello what do you think?

[behrad]

I also referenced the discussion topic about storing values inside Qlobber, which I was against, until @davedoesdev can provide us with a new fast built-in dedup Qlobber. (as fast as QlobberDedup) so we can remove codes like this

[GavinDmello]

@mcollina Could we just remove # from qlobber when he subscribes on a $SYS topic and vice versa ? So at a time we can't subscribe to both. I'm not sure if blocking offline storage for # would be right.

I'm 👍 for the QlobberDedup idea. It will give a huge performance boost

[davedoesdev]

@behrad do you have a dataset to benchmark? QosQlobber in the example I linked to is nearly the same as QlobberDedup. They both inherit from Qlobber but QosQlobber uses Maps and QlobberDedup uses Sets.

[davedoesdev]

I added to the benchmarks an example Qlobber (like QlobberQos in https://github.com/davedoesdev/qlobber/issues/9) but which stores its values in a Map. See https://github.com/davedoesdev/qlobber/blob/custom-qlobber-example/bench/options/_mapval.js

Results are below:

  add_match_remove x20,000
  ┌─────────┬────────────┬──────────────┬──────────┐
  │         │ total (ms) │ average (ns) │ diff (%) │
  ├─────────┼────────────┼──────────────┼──────────┤
  │ dedup   │      3,160 │      157,992 │        - │
  │ mapval  │      3,198 │      159,913 │        1 │
  │ default │      3,589 │      179,469 │       14 │
  └─────────┴────────────┴──────────────┴──────────┘

  match x20,000
  ┌─────────┬────────────┬──────────────┬──────────┐
  │         │ total (ms) │ average (ns) │ diff (%) │
  ├─────────┼────────────┼──────────────┼──────────┤
  │ mapval  │      2,649 │      132,466 │        - │
  │ dedup   │      2,719 │      135,950 │        3 │
  │ default │      3,049 │      152,468 │       15 │
  └─────────┴────────────┴──────────────┴──────────┘

Running "exec:bench-add-many" (exec) task

  add_many x1
  ┌─────────┬────────────┬───────────────┬──────────┐
  │         │ total (ms) │  average (ns) │ diff (%) │
  ├─────────┼────────────┼───────────────┼──────────┤
  │ dedup   │        299 │   298,539,115 │        - │
  │ mapval  │        397 │   397,112,585 │       33 │
  │ default │      5,112 │ 5,111,797,835 │    1,612 │
  └─────────┴────────────┴───────────────┴──────────┘

Running "exec:bench-match-many" (exec) task

  match_many x1
  ┌─────────┬────────────┬───────────────┬──────────┐
  │         │ total (ms) │  average (ns) │ diff (%) │
  ├─────────┼────────────┼───────────────┼──────────┤
  │ default │      5,251 │ 5,250,933,042 │        - │
  │ dedup   │      6,491 │ 6,491,299,224 │       24 │
  │ mapval  │      8,143 │ 8,142,677,353 │       55 │
  └─────────┴────────────┴───────────────┴──────────┘

[behrad]

do you have a dataset to benchmark?

I'll do a bench and let you know 👍

[behrad]

@davedoesdev based on your bench, will QosQlobber perform badly on wildcard topic publishes? (match-many) And can you add another bench like match-add where a topic is checked before adding, then added if not exists. (this will only be meaningful for default Qlobber)

[behrad]

Adding 2878969 topics @davedoesdev

Qlobber	time	heap
Qlobber	100886.361ms	3052.9MB
QlobberDedup	114514.373ms	3525.1MB
MapValQlobber	185898.631ms	3608.9MB

I think the best trade of is QlobberDedup, with single clientId values. With should do something to # topic handling @mcollina @GavinDmello

[mcollina]

@mcollina Could we just remove # from qlobber when he subscribes on a $SYS topic and vice versa ? So at a time we can't subscribe to both. I'm not sure if blocking offline storage for # would be right.

I do not think this will work, because qlobber will report # as matching $SYS. But if we can make it work 👍 . We can just refuse the current subscription, not the previous one.

[davedoesdev]

@behrad do you have the program for those benchmarks? I'd like to add it to the qlobber tests and see if I can improve things (although it might just be Map overhead).

[GavinDmello]

I do not think this will work, because qlobber will report # as matching $SYS. But if we can make it work 👍 . We can just refuse the current subscription, not the previous one.

I thought of removing any subscription with# from Qlobber and persistence when the client subscribes with a $SYS topic and vice versa so there would be changes in addSubscription, but rejecting the subscription also works. Let's document the behaviour though

[davedoesdev]

Is there anything I can do with Qlobber? I think MapValQlobber is a reasonable approach and doesn't require changing core. It seems Map has quite a bit more overhead than Set here.

[behrad]

I ran aedes with Redis persistence under my real-world load yesterday and unfortunately the hot path which was sticking cpu to 100% was https://github.com/mcollina/aedes-cached-persistence/blob/master/index.js#L45 as I expected.

This is no good for me, QlobberDedup helped me out in mosca, however in aedes we have the issue of keeping objects of subscriptions instead of clientIds in qlobber, only to do # handling! I will try both QLobberDedup (with clientId) and MapVal again under my real load, and let you know about the results. I believe we should go either way... next step is sharing a single cached-persistence process to factor all processing out inside a single shared one. (don't know if that even gets a bottleneck) but currently the publish-the-sub-receive-all-add-to-process is the scalability barrier, since a few tens of client connects per second is generating a huge CPU on all nodes of aedes all by executing Qlobber.add for the same each client. @mcollina @davedoesdev

[davedoesdev]

@behrad Are you still adding many entries with the same topic? If so, see https://github.com/davedoesdev/qlobber/issues/10#issuecomment-318278174

[davedoesdev]

I also have another idea. What if I added a test() method to Qlobber (and QlobberDedup)? Instead of collating all the matches, it would just check if one matches and return a boolean. The equality test could be a method, overridable for custom equality. This would mean checkSubsForClient wouldn't have to go through the collated matches.

[davedoesdev]

I've published qlobber version 0.12.2 which has a test(topic, val) which allows you to check whether a value is matched by a topic, without having to retrieve the whole match.

It seems to make a big difference:

  match_search_many x1
  ┌─────────┬────────────┬────────────────┬──────────┐
  │         │ total (ms) │   average (ns) │ diff (%) │
  ├─────────┼────────────┼────────────────┼──────────┤
  │ default │      1,743 │  1,743,358,885 │        - │
  │ dedup   │     11,237 │ 11,237,454,297 │      545 │
  │ mapval  │     33,800 │ 33,799,947,637 │    1,839 │
  └─────────┴────────────┴────────────────┴──────────┘

  test_many x1
  ┌─────────┬────────────┬──────────────┬──────────┐
  │         │ total (ms) │ average (ns) │ diff (%) │
  ├─────────┼────────────┼──────────────┼──────────┤
  │ default │          3 │    3,208,711 │        - │
  │ dedup   │          3 │    3,310,317 │        3 │
  │ mapval  │          3 │    3,312,106 │        3 │
  └─────────┴────────────┴──────────────┴──────────┘

match_search_many searches for values by matching first then going through the results.

test_many calls test() to do the same.

[davedoesdev]

If you change https://github.com/mcollina/aedes-cached-persistence/blob/master/index.js#L45 to call test().

You'll probably have to derive a custom Qlobber from QlobberDedup and override test_values to provide an equality function.

[behrad]

You'll probably have to derive a custom Qlobber from QlobberDedup and override test_values to provide an equality function

then the bottleneck would be moved into test_values , since the implementation should iterate over a Set of objects to see whose clientId matches. Am I right @davedoesdev ? Any fast data structure with direct access based on clientId would help, wouldn't?

Cpu consuming part is that there exist a number of wildcard topics, which all clients are subscribing to! And each restore/add subscription is checking trie to dedup before adding... And so it iterates all clients (a few millions) to dedup!!! @mcollina

[davedoesdev]

@behrad correct. But is it possible to customise the derived Qlobber to store indexed by clientId? Possibly a two tier structure by clientId and subscription? I'm going to take a look at the aedes code but won't be for a couple of days.

[behrad]

I am still on storing only clientId string in Qlobber and some how handling # topics... Storing subscription object is creating performance/complexity issues

[davedoesdev]

It would help to have a self-contained test we could benchmark outside aedes which replicates the issue.

[behrad]

I'll try to create one, yes!

What about storing a hash string like clientId,topic,qos inside QlobberDedup @davedoesdev ? considering that e.g. , is a separator that won't be used inside clientId and topic

matcher.add('foo/#', 'test1,foo/#,1') !?

[GavinDmello]

@behrad Have we considered stringifying ?

[behrad]

Didn't get it @GavinDmello ?

[davedoesdev]

What is it that needs looking up? What if you stored multiple chained hash tables e.g.

test1 -> foo/# -> 1 ?

[GavinDmello]

I meant instead of creating a custom hash, why not just insert json strings into Qlobberdedup. There would be some overhead parsing and stringifying but we can avoid using seperators

[behrad]

It would help to have a self-contained test we could benchmark outside aedes which replicates the issue.

In real world load, each aedes instance is restoring subs on whole client connects on the cluster (many more than joining/new clients that are adding new subscriptions). So a tons of .add calls will not add since qlobber contains the sub from the aedes _setup @ startup. Now @davedoesdev What dya propose in such scenario? DedupQlobber.add DedupQlobber.test + DedupQlobber.add DedupQlobber.match + DedupQlobber.add Qlobber.test + Qlobber.add Qlobber.match + Qlobber.add ... Again, considering that in most of cases, sub already exists and aedes won't add new subs inside trie. So i believe this is the hottest path.

[behrad]

What is it that needs looking up? What if you stored multiple chained hash tables e.g.

This is a good choice, Using QlobberDedup with clientId strings + storing a hash table inside aedes-cached-persistence as a side lookup to trie, If I'm getting you right?

this way we can take most of QlobberDedup, and also don't change aedes persistence API (we would be able to augment clientId with topic/qos (looking up the side hash table) when returning out. this looks more feasible/performant that the stringify or concat-hash that I proposed @GavinDmello , huh?

[behrad]

What is it that needs looking up?

sub trie matching would work only based on clientId as mosca @davedoesdev The only change in aedes related, is that aedes is returning the whole subscription (original topic is needed inside callers above). Client original subscribed topic will then be checked inside aedes router to ensure we are not routing a $SYS topic to a # topic! I want you to fix me if not guys @GavinDmello @mcollina

[davedoesdev]

@behrad @mcollina I'm keen to make a test case and benchmark in Qlobber. Could what aedes requires be put into such? I can try to guess and paste here if you like, but again it won't be immediately.

[behrad]

Why not to marking new/restore subs when publishing $SYS, then cluster nodes would be able to only call QlobberDedup.add for new subs! restore subs should be already added upon _setup. Is this ok @mcollina @GavinDmello ?

[mcollina]

@behrad can you flesh it out a bit more?

[behrad]

This is my BIG issue now under production, since we have multiple wildcard topics with 1.5 million clients. (3.5 million subscriptions) I will consider switching to QlobberDedup, and using an external hash table to store subscriptions (I hope the memory overhead be affordable) I will also take my optimization above into consideration in a new PR.