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

Control server thread #3349

Closed
wants to merge 31 commits into from
Closed

Conversation

lbudai
Copy link
Collaborator

@lbudai lbudai commented Jul 4, 2020

The aim of this PR is to help users when syslog-ng stuck on 'syslog-ng-ctl stats' (or on any other control command...).

There are two parts of the PR:

  • run ControlServer in a separate thread (it runs its own iv_main())
  • run stats/query commands in a separate thread

After this PR is merged, it is possible to stop syslog-ng with 'syslog-ng-ctl stop'
even if 'syslog-ng-ctl stats' is still in progress and 'syslog-ng-ctl stats' won't
'suspend' main thread anymore.

@lbudai lbudai added the bug label Jul 4, 2020
@lbudai lbudai added this to the syslog-ng-3.29 milestone Jul 4, 2020
@kira-syslogng
Copy link
Contributor

Build FAILURE

@lbudai
Copy link
Collaborator Author

lbudai commented Jul 6, 2020

@kira-syslogng : retest this please

@kira-syslogng
Copy link
Contributor

Build FAILURE

@kira-syslogng
Copy link
Contributor

Build FAILURE

@lbudai lbudai marked this pull request as draft July 6, 2020 15:24
@kira-syslogng
Copy link
Contributor

Build FAILURE

@kira-syslogng
Copy link
Contributor

Build FAILURE

@lbudai lbudai force-pushed the control-server-thread branch 2 times, most recently from 62eae72 to fa7139e Compare July 6, 2020 22:45
@kira-syslogng
Copy link
Contributor

Build FAILURE

2 similar comments
@kira-syslogng
Copy link
Contributor

Build FAILURE

@kira-syslogng
Copy link
Contributor

Build FAILURE

@furiel furiel self-requested a review July 7, 2020 06:27
lib/control/control-main.c Show resolved Hide resolved
lib/control/control-server.c Show resolved Hide resolved
lib/control/control-server.c Outdated Show resolved Hide resolved
lib/control/control-server.c Outdated Show resolved Hide resolved
lib/mainloop-control.c Show resolved Hide resolved
lib/secret-storage/secret-storage.c Outdated Show resolved Hide resolved
@lbudai lbudai force-pushed the control-server-thread branch 2 times, most recently from bcd88ad to 7d8dd9a Compare July 10, 2020 14:04
@kira-syslogng
Copy link
Contributor

Build FAILURE

@lbudai
Copy link
Collaborator Author

lbudai commented Jul 10, 2020

@kira-syslogng : retest this please

@kira-syslogng
Copy link
Contributor

Build FAILURE

@kira-syslogng
Copy link
Contributor

Build FAILURE

@lbudai lbudai force-pushed the control-server-thread branch 2 times, most recently from de2f031 to 3e1db0d Compare July 10, 2020 18:53
@kira-syslogng
Copy link
Contributor

Build FAILURE

@lbudai lbudai force-pushed the control-server-thread branch 2 times, most recently from ebc05b8 to 67c1437 Compare July 10, 2020 20:29
lbudai added 15 commits July 30, 2020 13:36
…ot running

ControlServer runs iv_main().
In unit tests, iv_main() is not running (dummy control server) and thus
iv_event_post has no effect - testcases won't receive response.

Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
After reload has done, send back the same response to the connected clients.

Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
ControlServer runs it's own iv_main() and when `control_connection_send_reply`
is called from a different thread, it executed update watches in the
wrong thread (in the main thread which caused a crash in iv_main).

Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
… thread

After ControlServer is running in a separate thread, secret storage calls
the async init cb from a different thread.

For example when password protected SSL cert is used and the password is set
by the user via 'syslog-ng-ctl credentials', we have to ensure that the
`afsocket_dd_try_connect` is executed in the main thread (there is a
main-loop-assert and there are ivykis objects registered during
`dd_try_connect`).

Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
…source

Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
TR_MAP_ASYNC_NOT_SUPPORTED means that callback is not registered and not
executed. If the caller wants, can execute the callback manually, or,
can do something else.

TR_MAP_ASYNC_REGISTERED_SUCCESS: callback registered successfully and
will be executed later

TR_MAP_ASYNC_REGISTERED_FAILES: failed to register callback

Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
`current_configuration` could become a dangling pointer when it is assigned
to `old_config` and later `old_config` is deinited - for this reason
we need to set it NULL after it is assigned to `old_config`.

`current_configuration` can be accessed concurrently, so we need to protect
it with a lock.

Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
As `reopen()` calls `main_loop_call` and playing with ivykis objects,
it should be run by the main thread.

Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
`control_connection_reopen()` and `control_connection_reload()` are the
same...

Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
one issue: as GThreads are not cancellable threads, join will be a
blocking call.

Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
@kira-syslogng
Copy link
Contributor

Build SUCCESS

…requested

Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
@kira-syslogng
Copy link
Contributor

Build SUCCESS

@lbudai
Copy link
Collaborator Author

lbudai commented Aug 12, 2020

this pr has been splitted into two smaller:
#3387
#3388

#3387 solves alone the blocking stats issue, we will check whether moving control server to a separate thread worth the complexity...

@lbudai lbudai closed this Aug 12, 2020
@szemere
Copy link
Collaborator

szemere commented Aug 12, 2020

@lbudai Thank You again for all the efforts to make it more "consumable" for us.

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

Successfully merging this pull request may close these issues.

None yet

6 participants