Subscribing from the enter_master block subscribes even the children.

raxoft commented 1 year ago

System Information

OS: Linux 4.15.0
Ruby: 2.7.6
Version: 0.7.52
OpenSSL: OpenSSL 1.1.1 11 Sep 2018

Description

I was trying to set up pub/sub in such a way so master can do some central processing of messages sent by workers before sending them to the connected clients. I used the on_state(:enter_master) block to subscribe the master, hoping it will be the only process which will receive the messages. But it turns out that also all but one children end up being subscribed as well.

Rack App to Reproduce

# I would hope this subscribes only the master.
Iodine.on_state(:enter_master) do
  # This is run several times in the master, once per each child - so far so good.
  puts "Process #{Process.pid} (master #{Iodine.master?}) started"
  Iodine.subscribe('test') do |source, data|
      # However this is run once in the master and once in each but one children.
      puts "Process #{Process.pid} (master #{Iodine.master?}) received #{data.inspect}"
  end
end

# This is needed to unsubscribe the children.
#Iodine.on_state(:enter_child) do
#  puts "Process #{Process.pid} (master #{Iodine.master?}) unsubscribing"
#  Iodine.unsubscribe('test')
#end

# Access the app to actually send single test message.
App = Proc.new do |env|
  Iodine.publish('test', 'ping')
  [200, {}, ['Message sent']]
end

run App

Testing code

curl http://localhost:3000/

Expected behavior

Only the master should be subscribed when subscribe is called from the :enter_master hook.

Actual behavior

It seems that most of the children (perhaps all but the first one spawned) are subscribed as well and receive the messages, despite the subscribe being called from master process only (albeit several times).

It can be currently worked around by using the :enter_child hook to unsubscribe the children, or one could test the master? flag in the called block, but I believe both such solutions are suboptimal and not what was originally intended.

boazsegev commented 1 year ago

Hi @raxoft ,

Thank you for opening this issue.

I'm reading through the code to track the behavior and yes, :enter_master blocks (or FIO_CALL_IN_MASTER callbacks) will be called multiple times - once after each time the master process spawns a worker.

The documentation about this might not be clear and I am sorry about that. I think this is mostly a naming and documentation issue.

Anyway, you are right - a subscription created by this block will be inherited by any child process after the first time the block was called.

Is subscription inheritance a feature or a bug?

This is true not only for when starting the server, but also when performing a hot restart (sending a USR1 signal sent to iodine) - than all the (new) workers will inherit all the master processes subscriptions.

This can't be changed without breaking the default subscription inheritance behavior (which arises from the nature of fork).

Since I'm now rewriting the code (working on version 0.8.x of the core library), I wonder if subscription inheritance is more of a problem than it is a feature and should I find a way for subscribe to prevent unwanted inheritance... making, perhaps, inheritance explicit.

Workaround

Of your suggested workaround solutions I liked this one the best:

# subscribe in master (called once).
Iodine.subscribe('test') do |source, data|
    puts "Process #{Process.pid} (master #{Iodine.master?}) received #{data.inspect}"
end
Iodine.on_state(:enter_child) do
  # unsubscribe in child (called only once when spawned).
  Iodine.unsubscribe('test')
end

raxoft commented 1 year ago

Ah, silly me. You are right, it all makes sense. I knew that the subscription is inherited, so I can't just subscribe at the top level, as that would be inherited to each worker. So I thought if I limit it to the :enter_master block, it will be called in master only after the workers are spawned. Then later I read that it is called for each worker spawned, but at that point I didn't connect it with the inheritance and that the subsequent forks will inherit the prior subscription. That's why it is n-1, not n. And you are right that even if the block was called only once, the subsequent respawns of future workers would still inherit the subscription...

Well, your suggested solution works fine for me. It's the same thing which one does when plumbing the file descriptors in parent and child process after fork. I should have realized it myself. However, regarding the possible enhancement you mention, you can add a flag which would work similar to FD_CLOEXEC for file descriptors. That's quite handy feature that saves quite a lot of troubles after forking and execing something. In your case it would mean after fork in the child process going through all subscriptions and unsubscribing those which are marked that they should not be inherited. It's essentially what the workaround does, but has the advantage that it doesn't require explicit knowledge of all subscriptions which might exist prior the fork.

In either case, thanks for explaining the behavior and sorry for bothering you. It's all clear now so feel free to close this at any time. Thanks.

boazsegev commented 1 year ago

Hi @raxoft ,

I'm already working on a solution for the 0.8.x version. Thanks for pointing out this use-case 👍🏻

B.

boazsegev / iodine