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

Keep the program destination open between configuration reloads #722

Merged
merged 10 commits into from
Nov 4, 2015
Merged

Keep the program destination open between configuration reloads #722

merged 10 commits into from
Nov 4, 2015

Conversation

nvxxu2i
Copy link
Contributor

@nvxxu2i nvxxu2i commented Oct 1, 2015

This pull request adds the possibility of letting the program destination unclosed and reused between configuration reloads.

New option: keep_alive(x), where x is either yes or no.
The default behaviour has not been changed.

Review on Reviewable

@@ -112,6 +113,7 @@ dest_afprogram_options
dest_afprogram_option
: dest_writer_option
| dest_driver_option
| KW_KEEPALIVE '(' yesno ')' { ((AFProgramDestDriver *)last_driver)->keep_alive = $3; }
;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use a setter in this case, this is the preferred pattern over mucking within the private struct of the driver.

Also, keepalive is not the the best name, we are using keep-alive (e.g. with the dash in the socket module, so we should use the same).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d143a96270607999fba326659cd00f1f6a5d70b4.

@nvxxu2i nvxxu2i self-assigned this Oct 5, 2015
@@ -38,6 +38,45 @@
#include <unistd.h>
#include <signal.h>

typedef struct _AFProgramReloadStoreItem
{
LogWriter * writer;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vertical alignment

MÓZES Ádám István added 10 commits November 4, 2015 13:26
Signed-off-by: Fried Zoltán <zoltan.fried@balabit.com>
Signed-off-by: MÓZES Ádám István <adam.mozes@balabit.com>
The writer deinitialization should be happen before the
termination of the child, because at the end of the
deinitialization, a force flush can be happen, and if the
child is terminated before the force flush, a SIGPIPE can
cause the recreation of the destination program.

Signed-off-by: MÓZES Ádám István <adam.mozes@balabit.com>
This option will control whether the destination program should be
terminated at reload, or should be left running.

Signed-off-by: MÓZES Ádám István <adam.mozes@balabit.com>
This commit introduces an additional abstraction layer to the child terminating process, making it
unified.

Signed-off-by: MÓZES Ádám István <adam.mozes@balabit.com>
This commit will add the AFProgramReloadStoreItem structure
and a basic free function. This structure will hold essential
information about the program destination,
which allows us to restore it later.

Signed-off-by: MÓZES Ádám István <adam.mozes@balabit.com>
This commit will add a simple implementation of restoring the
AFProgramReloadStoreItem structure when syslog-ng is being reloaded.

Signed-off-by: MÓZES Ádám István <adam.mozes@balabit.com>
This commit contains a naive implementation of the
AFProgramReloadStoreItem saving logic when the program destination
is being deinitalized.

Signed-off-by: Fried Zoltán <zoltan.fried@balabit.com>
Signed-off-by: MÓZES Ádám István <adam.mozes@balabit.com>
The child_manager_unregister has been moved to
afprogram_dd_deinit in order to restore the single
responsibility of the afprogram_dd_kill_child.

Signed-off-by: MÓZES Ádám István <adam.mozes@balabit.com>
This commit will extract the program opening logic into a separate
method (called afprogram_dd_open_program), therefore this is merely a
refactor commit.

Signed-off-by: MÓZES Ádám István <adam.mozes@balabit.com>
The LogQueue of the writer had the same name as the ReloadStoreItem,
which is clearly a mistake.
The impact of the issue was quite low, because it appeared rarely.

Signed-off-by: MÓZES Ádám István <adam.mozes@balabit.com>
@nvxxu2i
Copy link
Contributor Author

nvxxu2i commented Nov 4, 2015

The marked commits have been fixed up, the following differences should be the same:

lbudai added a commit that referenced this pull request Nov 4, 2015
Keep the program destination open between configuration reloads
@lbudai lbudai merged commit 5093378 into syslog-ng:master Nov 4, 2015
@nvxxu2i nvxxu2i deleted the program-stay-alive branch November 4, 2015 20:49
@nvxxu2i nvxxu2i removed their assignment Nov 4, 2015
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 this pull request may close these issues.

None yet

5 participants