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

[WIP] Expedite threaded flush at reload #2656

Open
wants to merge 8 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@bazsi
Copy link
Member

commented Apr 3, 2019

This branch contains some of the preparatory steps for #2496 , splitting of the bulk of the branch for easier review and integration, asked by @furiel.

This contains the "expedited" flush implemented for the http() destination, that will also be used by the upcoming kafka destination.

It also contains some dbld/makefile improvements.

With that this becomes a dependency of #2496

@kira-syslogng

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2019

Build FAILURE

@furiel

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2019

@bazsi thank you for the split

@Kokan Kokan added this to the syslog-ng-3.21 milestone Apr 11, 2019

Kokan and others added some commits Oct 4, 2018

make: sort modules include
Signed-off-by: Kokan <kokaipeter@gmail.com>
dbld: pass CONFIGURE_OPTS to all docker invocations
This makes it possible to run the bootstrap script from a launched docker
shell.

Signed-off-by: Balazs Scheidler <balazs.scheidler@oneidentity.com>
logthrdestdrv: add time_reopen value to log messages where a retry is…
… upcoming

Signed-off-by: Balazs Scheidler <balazs.scheidler@oneidentity.com>
mainloop: publish main_loop_is_terminating()
In order to differentiate between a reload and a shutdown in the
deinit() path, publish this function.

Signed-off-by: Balazs Scheidler <balazs.scheidler@oneidentity.com>
cfg: add cfg_is_shutting_down() function
This function allows drivers to determine if we are deinitializing
for a reload or a shutdown. At shutdown we should do a complete flush
whereas during reload we can be less strict as the LogQueue instances
remain intact. At shutdown we only retain the messages in the
queue if the user is using a disk queue.

Signed-off-by: Balazs Scheidler <balazs.scheidler@oneidentity.com>

@bazsi bazsi force-pushed the bazsi:expedite-threaded-flush-at-reload branch from f8f222d to 735d83b Apr 12, 2019

@bazsi

This comment has been minimized.

Copy link
Member Author

commented Apr 12, 2019

I have now rebased to current master and also made an attempt to fix the compilation errors. (Some of
the filenames were incorrect at the point of the splitting of the branch).

Show resolved Hide resolved lib/logthrdest/tests/Makefile.am Outdated
Show resolved Hide resolved modules/http/http-worker.c
@kira-syslogng

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

Build FAILURE

@bazsi bazsi force-pushed the bazsi:expedite-threaded-flush-at-reload branch from 735d83b to 6541dbb Apr 12, 2019

Show resolved Hide resolved lib/logthrdest/CMakeLists.txt Outdated
@kira-syslogng

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

Build FAILURE

@bazsi bazsi force-pushed the bazsi:expedite-threaded-flush-at-reload branch from 6541dbb to b58f0ee Apr 12, 2019

@kira-syslogng

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

Build FAILURE

Show resolved Hide resolved lib/logthrdest/CMakeLists.txt Outdated
Show resolved Hide resolved lib/logthrdest/CMakeLists.txt Outdated

bazsi added some commits Mar 14, 2019

logthrdestdrv: move logthrdestdrv to a separate directory under lib
Signed-off-by: Balazs Scheidler <balazs.scheidler@oneidentity.com>
logthrdestdrv: add "expedite" option to flush
The new option makes it possible to know that we are going down for a reload.
If expedite is TRUE, we shouldn't take much time to perform the flush
operation, rather we can simply return LTR_RETRY to indicate
that we want to reprocess the entire batch once the new config
is initialized.

This should speed up reload operations significantly if the target
server is slow.

Signed-off-by: Balazs Scheidler <balazs.scheidler@oneidentity.com>
http: abort the current batch in case we are going down for a reload
In case of a reload (vs. a shutdown) we can easily drop the current batch
as that will wait for us nicely in the queue when we start back up.

At shutdown we are not so fortunate, as we will only be able to retain
the messages if the user has a disk buffer.

Signed-off-by: Balazs Scheidler <balazs.scheidler@oneidentity.com>

@bazsi bazsi force-pushed the bazsi:expedite-threaded-flush-at-reload branch from b58f0ee to d30c961 Apr 14, 2019

@kira-syslogng

This comment has been minimized.

Copy link
Contributor

commented Apr 14, 2019

Build FAILURE

@furiel
Copy link
Contributor

left a comment

I do not have any blocking request on the code. The only strange thing is cfg_is_shutting_down, that does not have any connection with cfg, so I proposed that to turn it into void. But that also looks strange, because it would look a typical method of a class, just without self. I do not know how to resolve it nicely, so if you feel so, you can leave as it is.

PS: http destination tests are failing (maybe related to expledite flush?). If the tests are fixed, this pr is ready to go in from my point of view.

Show resolved Hide resolved lib/cfg.c
Show resolved Hide resolved lib/logthrdest/logthrdestdrv.c
@bazsi

This comment has been minimized.

Copy link
Member Author

commented Apr 15, 2019

@pzoleex

This comment has been minimized.

Copy link
Contributor

commented Apr 15, 2019

@kira-syslogng test this please test branch=pzolee-expedite-threaded-flush-at-reload;

@kira-syslogng

This comment has been minimized.

Copy link
Contributor

commented Apr 15, 2019

Build SUCCESS

@lbudai lbudai self-requested a review Apr 15, 2019

Waiting for ENUM-ing.

@kira-syslogng kira-syslogng changed the title Expedite threaded flush at reload [WIP] Expedite threaded flush at reload Apr 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.