Bug in SpecificDiscoveryStore breaks PubSub communication

[KonradBreitsprecherBkd]

For a specific setup, the PubSub communication is failing. Minimal conditions to reproduce the bug are:

• Two publishers, one subscriber (all running autonomous, async)
• The publishers carry the optional label L1
• The single subscriber carries the optional label L1 and the optional label L2
• The publishers are started first, then the subscriber is started

// Start the two publishers first
.\SilKitDemoPublisher.exe -Aa -n Pub1 -f
.\SilKitDemoPublisher.exe -Aa -n Pub2 -f

// Then start the subscriber
.\SilKitDemoSubscriber.exe -aA

// Bug: No reception on the subscriber

The bug can be traced back to the SpecificDiscoveryStore. This is the lookup algorithm that is supposed to improve the matching of publishers and subscribers with labels.

[VJanKraemer]

So this just matches the last label to have any nodes in the cluster instead of the one with the least nodes?

[KonradBreitsprecher]

I think we need the matchCount condition and > 0

[KonradBreitsprecher]

I think we need the matchCount condition and > 0. The bug was that the outgreedylabel that was found here in the specific label setup (reproduced by the test) did not contain any handers (of the subscriber) and thus the follow up logic to finish the pubsub connection never happened.

Maybe the dict here should never have been populated to get into this situation, at least the >0 prevents the bug.

Also, for "symmetry reasons" there might be the same situation for mandatory Labels a few lines above.

To stir it up a little more, we might need at least one person who proudly says "I understand what's happening here" otherwise we have a black box algorithm in a central unit that was "introduced for performance reasons by a former colleague". If there is no xkcd we should make one...

[MariusBgm]

I think we need the matchCount condition and > 0. The bug was that the outgreedylabel that was found here in the specific label setup (reproduced by the test) did not contain any handers (of the subscriber) and thus the follow up logic to finish the pubsub connection never happened.

Maybe the dict here should never have been populated to get into this situation, at least the >0 prevents the bug.

Also, for "symmetry reasons" there might be the same situation for mandatory Labels a few lines above.

To stir it up a little more, we might need at least one person who proudly says "I understand what's happening here" otherwise we have a black box algorithm in a central unit that was "introduced for performance reasons by a former colleague". If there is no xkcd we should make one...

I fully agree - I think i'll try to make a clean room implementation of the label matching code next week.
This PR is no show stopper for the upcoming 5.0.3 release, though.

[KonradBreitsprecher]

That sounds great. You can use https://github.com/vectorgrp/sil-kit/blob/main/SilKit%2FIntegrationTests%2FFTest_PubSubPerf.cpp to check performance degradation. I think there's a commented test set for production that runs a while but pushes the pubsub code to it's limits..

[KonradBreitsprecher]

That sounds great. You can use https://github.com/vectorgrp/sil-kit/blob/main/SilKit%2FIntegrationTests%2FFTest_PubSubPerf.cpp to check performance degradation. I think there's a commented test set for production that runs a while but pushes the pubsub code to it's limits..