Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin hook ordering #4005

Closed
cdecker opened this issue Sep 3, 2020 · 1 comment · Fixed by #4168
Closed

Plugin hook ordering #4005

cdecker opened this issue Sep 3, 2020 · 1 comment · Fixed by #4168

Comments

@cdecker
Copy link
Member

cdecker commented Sep 3, 2020

@m-schmoock points out that the order in which hooks are added to the chain,
and processed is not guaranteed to match the order in which the plugins are
specified on the command line, and may be randomly interleaved.

The current theory is that this happens due to the concurrent call to
getmanifest and the order in which the response gets processed, i.e., the
order in which the hooks get added to the chain. In order to address this
issue we should ensure that the hook order matches the order in which the
corresponding plugin was specified on the command line. This can be achieved
in several ways:

  • Call the getmanifest sequentially. Pros: simple, cons: delays startup time.
  • Process the getmanifest results in the plugin order. Pros: also
    relatively simple, cons: requires stashing of the results and processing in
    a non-request-response manner, memory overhead.
  • Assign each hook registration a priority based on the plugin order, and
    sort before calling init on the plugin. Pros: simple, fast. Cons: adds a
    tiny bit of computation and memory overhead.

I personally tend to using the third option, since it is quick and has low
overhead. In addition we can use the priority in future to either allow users
to specify explicit priorities on the command line, or the plugins to specify
priorities for each hook independently, freeing us from the cli order and
adding flexibility. However these additions are likely out of scope for this
issue and need to be refined.

Details

@rustyrussell
Copy link
Contributor

Explicit ordering is always better, in future some plugins may need to be ordered with respect to each other. There are several ways of doing this. Simplest is a priority number. Best is before and after lists.

So, the ideal is:

  1. Allow before and after lists in hook registration.
  2. Do partial order sort by those.
  3. Secondary sort by command line order, then config order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants