Add ZMQ_SUBSCRIPTION_COUNT socket option

zeromq / libzmq

ZeroMQ core engine in C++, implements ZMTP/3.1

https://www.zeromq.org

Mozilla Public License 2.0

9.71k stars 2.35k forks source link

Add ZMQ_SUBSCRIPTION_COUNT socket option #4459

Closed f18m closed 1 year ago

f18m commented 1 year ago

Hi @bluca , can you review this PR ? I think it's ready to go, EXCEPT for the documentation. But before adding that I would like to have some feedback if the way this (simple) feature is implemented is correc.t.. thanks

bluca commented 1 year ago

This is going to be racy - subs are managed in the i/o thread(s), but the option is read from the application thread. At least make it use atomics.

f18m commented 1 year ago

This is going to be racy - subs are managed in the i/o thread(s), but the option is read from the application thread. At least make it use atomics.

good point. I'll change it to use atomics, thanks for the review

f18m commented 1 year ago

@bluca , can you go over this PR again? I have added use of atomics, improve the test_pubsub_num_subscriptions by testing also "nested" prefixes (subscriptions sharing the same initial bytes), and docs.

open points:

should we implement ZMQ_SUBSCRIPTION_COUNT also for SUB sockets?
documentation of zmq_getsocktopt() seems to be in alphabetical order in the past but now it's not anymore... I can raise another PR later on to reorder the entries if that's useful

bluca commented 1 year ago

CMake Error at tests/CMakeLists.txt:252 (add_executable):
  Cannot find source file:

    test_pubsub_num_subscriptions.cpp

  Tried extensions .c .C .c++ .cc .cpp .cxx .cu .mpp .m .M .mm .ixx .cppm .h
  .hh .h++ .hm .hpp .hxx .in .txx .f .F .for .f77 .f90 .f95 .f03 .hip .ispc

CMake Error at tests/CMakeLists.txt:252 (add_executable):
  No SOURCES given to target: test_pubsub_num_subscriptions

f18m commented 1 year ago

CMake Error at tests/CMakeLists.txt:252 (add_executable):
  Cannot find source file:

    test_pubsub_num_subscriptions.cpp

  Tried extensions .c .C .c++ .cc .cpp .cxx .cu .mpp .m .M .mm .ixx .cppm .h
  .hh .h++ .hm .hpp .hxx .in .txx .f .F .for .f77 .f90 .f95 .f03 .hip .ispc

CMake Error at tests/CMakeLists.txt:252 (add_executable):
  No SOURCES given to target: test_pubsub_num_subscriptions

thanks - should be fixed now (just a bit hard to understand the CI/CD results, and github does not help marking every single commit with "CI failed")