dbus-broker icon indicating copy to clipboard operation
dbus-broker copied to clipboard

enable dbus-broker reexecute

Open Geass-LL opened this issue 2 years ago • 5 comments

This add a new feature to dbus-broker. With this patch, we can reexecute dbus-broker by running dbus-broker-launch --reexec.

The main idea behind this commit is:

  1. Serialize: when broker gets the Reexecute message, it will serialize the main information (socket fd, id, uid, pid, match rule, request name, sasl) to the anonymous file,
  2. Reexecute: launcher and broker reexecute.
  3. AddListener: when launcher has reexecuted successfully, it will send AddListener to broker.
  4. Recover: broker recovers all peers when processing AddListener message, and it uses the socket fd saved in anonymous file to create new peers, and recover id/uid/pid/...
  5. Ready: launcher will broadcast Ready signal, when the dbus-broker-launch --reexec gets this signal, the reexecuting process is over.

I'll split this commit to several small commits after your first review. Please take a look, thank a lot.

Geass-LL avatar Jan 05 '23 03:01 Geass-LL

Thank you for working on this! It is our most requested feature and will surely be greatly appreciated by many.

Some questions before I look at the code in detail (I am sure I'll think of more things in the coming days):

  • How do you version the serialized data, and what kinds of upgrade paths do you think we should support? My suggestion: save a major.minor version for the serialized state and increment the minor number when we make a change that can not be rolled back from and change the major number when we make changes that we cant roll forward to (nor back from). Then fail to reexec if we find an incompatible version number.
  • How do you think we should deal with in-flight messages? If I understand correctly they are now currently dropped? I think we can't be lossy, so need to come up with something. Ideally we would serialize everything, but we could also possibly consider some sort of drain operation.

teg avatar Jan 05 '23 10:01 teg

Regarding the draining, it might be worth while to introduce a Freeze and Thaw call and only allow reexec when the broker is frozen. I think this might simplify the proposed reexec logic as an added bonus (as we would still allow calls on the launcher API while frozen).

Freeze would finish any partial dispatch and stop further dispatching. We still need to serialize messages that are queued but not dispatched though. An open question is what to do about misbehaving peers who have stopped reading/worrying from/to their sockets.

teg avatar Jan 05 '23 10:01 teg

  • How do you version the serialized data, and what kinds of upgrade paths do you think we should support? My suggestion: save a major.minor version for the serialized state and increment the minor number when we make a change that can not be rolled back from and change the major number when we make changes that we cant roll forward to (nor back from). Then fail to reexec if we find an incompatible version number.

I didn't think about the version. Currently, I only serialize the peer's fd/id/uid/pid/requested name/match rule/sasl and broker's max_id to know which id we should distribute to the newly connected peer after reexecuting. The serialized string is like this:

max_ids=1234
peer_str=12;5600;91750;996;org.freedesktop.PolicyKit1;[{type='signal',sender='org.freedesktop.DBus',interface='org.freedesktop.DBus',member='NameOwnerChanged',path='/org/freedesktop/DBus'}];[{1}{1}{0}]
  • 12: the socket fd of this peer
  • 5600: the PID of this peer
  • 91750: the peer ID :1.91750
  • 996: the UID of this peer
  • org.freedesktop.PolicyKit1: the requested name
  • [{type='signal'...]: the match rule
  • [{1}{1}{0}]: the sasl state

These should be enough to recover a peer. But yes, we may have some big changes in the future version, major.minor version sounds a good idea. After rpm -Uvh dbus-broker.aarch64, dbus-broker-launch --reexec can send its version to the running old broker, then broker compares the received version with its own, if the two versions are incompatible, the broker refuses to reexecute and replies an error.

  • How do you think we should deal with in-flight messages? If I understand correctly they are now currently dropped? I think we can't be lossy, so need to come up with something. Ideally we would serialize everything, but we could also possibly consider some sort of drain operation.

I don't close the peer's fd during reexecuting, and write all messages out before reexecuting. After reexecuting, I reepoll these fd and process it. I tested this by the following testcase:

  1. Create a server which can accept two 'x' numbers, and reply the multiply result as 'x';
  2. Create a server which sends method_call to the server in a busy loop, and check the reply;
  3. Reexecuting dbus-broker in a busy loop background for 1000 times;

No message was lost.

Currently, this commit doesn't support create a new connection during reexecuting.

Geass-LL avatar Jan 06 '23 01:01 Geass-LL

This is really cool - one suggestion, maybe use systemd's FD store? That should allow to avoid needing to manually handle things, and a restart should "just" work?

bluca avatar Jan 15 '23 13:01 bluca

This is really cool - one suggestion, maybe use systemd's FD store? That should allow to avoid needing to manually handle things, and a restart should "just" work?

Thanks for your suggestion, I'll look into it.

Geass-LL avatar Jan 16 '23 02:01 Geass-LL