gluster-block: add logrotate support #163

lxbsz · 2018-11-19T02:42:31Z

Fixes: issue #164

Signed-off-by: Xiubo Li [email protected]

pkalever

rest looks good to me @lxbsz

systemd/gluster-block.logrotate

pkalever · 2019-03-20T10:27:32Z

@lxbsz can you please rebase this on current ?

lxbsz · 2019-03-20T10:40:38Z

@lxbsz can you please rebase this on current ?

Sure, will do it later.
Thanks.

lxbsz · 2019-03-24T00:51:39Z

@pkalever @amarts
Updated it.
Thanks.

systemd/gluster-block.logrotate

lxbsz · 2019-03-28T09:12:24Z

@pkalever This has been already fixed, please see:

dd84e7d

Thanks.

lxbsz · 2019-03-29T04:21:43Z

@pkalever
Fix them all, please review.
Thanks.

pkalever

@lxbsz we cannot watch on /etc/sysconfig. We need to create a new config dir under like /etc/sysconfig/gluster-block/ and then move the conf file under it. Then we can watch on /etc/sysconfig/gluster-block/ directory

HTH

lxbsz · 2019-04-02T04:25:44Z

@lxbsz we cannot watch on /etc/sysconfig. We need to create a new config dir under like /etc/sysconfig/gluster-block/ and then move the conf file under it. Then we can watch on /etc/sysconfig/gluster-block/ directory

@pkalever
As we discussed days ago, it is watching the /etc/sysconfig/ directory and will the filter out whether the gluster-blockd config file is changed.

So please review.
Thanks.

pkalever · 2019-04-02T04:27:10Z

@lxbsz we cannot watch on /etc/sysconfig. We need to create a new config dir under like /etc/sysconfig/gluster-block/ and then move the conf file under it. Then we can watch on /etc/sysconfig/gluster-block/ directory

@pkalever
As we discussed days ago, it is watching the /etc/sysconfig/ directory and will the filter out whether the gluster-blockd config file is changed.

So please review.

@lxbsz This was in my list yesterday, got skipped coz I worked on something else.
I will provide the review today.

Thanks!

pkalever

@lxbsz have left few review suggestions, please take a look. Thanks!

utils/dyn-config.c

systemd/gluster-blockd.sysconfig

utils/utils.c

systemd/gluster-blockd.sysconfig

utils/dyn-config.c

configure.ac

utils/utils.c

cli/gluster-block.c

utils/dyn-config.c

pkalever · 2019-04-02T10:58:10Z

@lxbsz also can this series accommodate leak reported at #183

lxbsz · 2019-04-03T04:08:33Z

@pkalever Updated it. Thanks.

lxbsz · 2019-04-03T05:21:55Z

@lxbsz also can this series accommodate leak reported at #183

There is no any memory leakage here, I updated the issue#183.

Thanks.

pkalever

@lxbsz this series looks much simplified.

Do you want to send below patches in a separate PR ?

config: logDir add dyn support
logrotate: update the logdir path whenever it changes
dyn-cfg: skip resetting options if they match with current

Thanks!

configure.ac

extras/Makefile.am

gluster-block.spec.in

utils/dyn-config.c

lxbsz · 2019-04-03T07:49:02Z

@lxbsz this series looks much simplified.

Do you want to send below patches in a separate PR ?

config: logDir add dyn support

logrotate: update the logdir path whenever it changes

dyn-cfg: skip resetting options if they match with current

Yeah, let's add this after we resolved the -glfs.log issue. Or the logDir dyn make no sense.

Thanks!

pkalever · 2019-04-03T07:55:12Z

@lxbsz this series looks much simplified.
Do you want to send below patches in a separate PR ?

config: logDir add dyn support

logrotate: update the logdir path whenever it changes

dyn-cfg: skip resetting options if they match with current

Yeah, let's add this after we resolved the -glfs.log issue. Or the logDir dyn make no sense.

Yeah, this is one simple thing to do.
We can fetch the all glfs objects from LRU cache and set:

for each glfs in lruCaheList:
  ret = glfs_set_logging(glfs, ${dynamic-logDir}/gluster-block-gfapi.log", 7);

lxbsz · 2019-04-03T07:59:38Z

@lxbsz this series looks much simplified.
Do you want to send below patches in a separate PR ?

config: logDir add dyn support

logrotate: update the logdir path whenever it changes

dyn-cfg: skip resetting options if they match with current

Yeah, let's add this after we resolved the -glfs.log issue. Or the logDir dyn make no sense.

Yeah, this is one simple thing to do.
We can fetch the all glfs objects from LRU cache and set:
for each glfs in lruCaheList:
  ret = glfs_set_logging(glfs, ${dynamic-logDir}/gluster-block-gfapi.log", 7);

As I remembered months ago, after the volume is inited by glfs_init, there will be a problem with this.
Not very sure, I need much more test.

Thanks.

pkalever · 2019-04-03T08:11:19Z

As I remembered months ago, after the volume is inited by glfs_init, there will be a problem with this.
Not very sure, I need much more test.

I tested this locally (with very few changes to glfs-tests.c ) and it works for me.
If yo notice anything specific, you should file it.

Thanks!

lxbsz · 2019-04-03T08:28:10Z

As I remembered months ago, after the volume is inited by glfs_init, there will be a problem with this.
Not very sure, I need much more test.

I tested this locally (with very few changes to glfs-tests.c ) and it works for me.
If yo notice anything specific, you should file it.

Okay, cool.
Let's do this in another PR and also we need to fix this in tcmu-runner at the same.

BRs

Thanks!

pkalever · 2019-04-03T09:12:57Z

I tested this locally (with very few changes to glfs-tests.c ) and it works for me.
If yo notice anything specific, you should file it.

Okay, cool.
Let's do this in another PR and also we need to fix this in tcmu-runner at the same.

I'm okay moving this part to next PR. Can you just raise a issue for this, so that we don't miss out ?
Please respin this series with agreed changes

Thanks!

lxbsz · 2019-04-03T09:16:32Z

I tested this locally (with very few changes to glfs-tests.c ) and it works for me.
If yo notice anything specific, you should file it.

Okay, cool.
Let's do this in another PR and also we need to fix this in tcmu-runner at the same.

I'm okay moving this part to next PR. Can you just raise a issue for this, so that we don't miss out ?

Please respin this series with agreed changes

Sure. #199

pkalever · 2019-04-03T11:40:56Z

Sample output:
[root@Fedora-Server gluster-block]# ls -l /var/log/gluster-block
total 774728
0001-logs-remove-n-in-LOG-usage.patch.txt

-rw-r--r--. 1 root root 9356 Mar 29 22:36 cmd_history.log
-rw-r--r--. 1 root root 1356 Mar 29 19:00 gluster-block-cli.log
-rw-r--r--. 1 root root 9298 Mar 29 22:36 gluster-block-configshell.log
-rw-r--r--. 1 root root 1928 Apr 3 16:49 gluster-blockd.log
-rw-r--r--. 1 root root 737029075 Apr 3 16:22 gluster-blockd.log.1
-rw-r--r--. 1 root root 56215936 Apr 3 16:18 gluster-blockd.log.2.gz
-rw-------. 1 root root 39118 Mar 29 22:36 gluster-block-gfapi.log
[root@Fedora-Server gluster-block]#

Reviewed & Tested-by: Prasanna Kumar Kalever <[email protected]>

@lxbsz please update the tags.

Also can you consider this patch to be attached as part of this PR:
0001-logs-remove-n-in-LOG-usage.patch.txt

Without this patch the dyn-config logs looks quite clumsy for me.

Thanks!

Signed-off-by: Xiubo Li <[email protected]> Reviewed-by: Prasanna Kumar Kalever <[email protected]> Tested-by: Prasanna Kumar Kalever <[email protected]>

Editors (such as vim, nano ..) follow different approaches to save conf file. The two commonly followed techniques are to overwrite the existing file, or to write to a new file (.swp, .tmp ..) and move it to actual file name later. In the later case, the inotify fails, because the file it's been intended to watch no longer exists, as the new file is a different file with just a same name. To handle both the file save approaches mentioned above, it is better we watch the directory and filter for MODIFY events. Signed-off-by: Xiubo Li <[email protected]> Reviewed-by: Prasanna Kumar Kalever <[email protected]> Tested-by: Prasanna Kumar Kalever <[email protected]>

Signed-off-by: Xiubo Li <[email protected]> Reviewed-by: Prasanna Kumar Kalever <[email protected]> Tested-by: Prasanna Kumar Kalever <[email protected]>

Signed-off-by: Prasanna Kumar Kalever <[email protected]>

lxbsz · 2019-04-03T12:25:52Z

Sample output:
[root@Fedora-Server gluster-block]# ls -l /var/log/gluster-block
total 774728
0001-logs-remove-n-in-LOG-usage.patch.txt

-rw-r--r--. 1 root root 9356 Mar 29 22:36 cmd_history.log
-rw-r--r--. 1 root root 1356 Mar 29 19:00 gluster-block-cli.log
-rw-r--r--. 1 root root 9298 Mar 29 22:36 gluster-block-configshell.log
-rw-r--r--. 1 root root 1928 Apr 3 16:49 gluster-blockd.log
-rw-r--r--. 1 root root 737029075 Apr 3 16:22 gluster-blockd.log.1
-rw-r--r--. 1 root root 56215936 Apr 3 16:18 gluster-blockd.log.2.gz
-rw-------. 1 root root 39118 Mar 29 22:36 gluster-block-gfapi.log
[root@Fedora-Server gluster-block]#

Reviewed & Tested-by: Prasanna Kumar Kalever <[email protected]>

@lxbsz please update the tags.

Also can you consider this patch to be attached as part of this PR:
0001-logs-remove-n-in-LOG-usage.patch.txt

Without this patch the dyn-config logs looks quite clumsy for me.

Yeah, updated it.
Thanks.

Thanks!

pkalever · 2019-04-03T12:52:06Z

@lxbsz Thanks!

ghost assigned lxbsz Nov 19, 2018

ghost added the in progress label Nov 19, 2018

pkalever requested changes Nov 29, 2018

View reviewed changes

systemd/gluster-block.logrotate Outdated Show resolved Hide resolved

systemd/gluster-block.logrotate Outdated Show resolved Hide resolved

lxbsz force-pushed the logrotate branch 2 times, most recently from 538a026 to 5771283 Compare November 30, 2018 06:38

lxbsz force-pushed the logrotate branch 2 times, most recently from af0b9af to dd84e7d Compare March 24, 2019 00:48

pkalever requested changes Mar 25, 2019

View reviewed changes

systemd/gluster-block.logrotate Outdated Show resolved Hide resolved

lxbsz mentioned this pull request Mar 25, 2019

dyn-logger: make sure the logLevel changing log visible always #177

Closed

lxbsz mentioned this pull request Mar 29, 2019

dyn-cfg: Avoid putting large char array on the stack #186

Closed

lxbsz force-pushed the logrotate branch from dd84e7d to e4d8e3e Compare March 29, 2019 03:23

pkalever requested changes Mar 29, 2019

View reviewed changes

pkalever requested changes Apr 2, 2019

View reviewed changes

lxbsz force-pushed the logrotate branch 2 times, most recently from 8ee872d to cbe7341 Compare April 3, 2019 04:07

lxbsz force-pushed the logrotate branch 2 times, most recently from 9bc7167 to fb0f7ac Compare April 3, 2019 04:56

pkalever reviewed Apr 3, 2019

View reviewed changes

configure.ac Show resolved Hide resolved

extras/Makefile.am Show resolved Hide resolved

gluster-block.spec.in Show resolved Hide resolved

utils/dyn-config.c Outdated Show resolved Hide resolved

lxbsz force-pushed the logrotate branch from fb0f7ac to 0b99d3a Compare April 3, 2019 07:51

pkalever approved these changes Apr 3, 2019

View reviewed changes

lxbsz and others added 4 commits April 3, 2019 20:22

logger: add CRIT loglevel support

7c1e2a9

Signed-off-by: Xiubo Li <[email protected]> Reviewed-by: Prasanna Kumar Kalever <[email protected]> Tested-by: Prasanna Kumar Kalever <[email protected]>

gluster-block: add logrotate support

011366b

Signed-off-by: Xiubo Li <[email protected]> Reviewed-by: Prasanna Kumar Kalever <[email protected]> Tested-by: Prasanna Kumar Kalever <[email protected]>

logs: remove '\n' in LOG() usage

5f0e1b8

Signed-off-by: Prasanna Kumar Kalever <[email protected]>

lxbsz force-pushed the logrotate branch from 0b99d3a to 5f0e1b8 Compare April 3, 2019 12:25

pkalever merged commit 5c5ebf0 into gluster:master Apr 3, 2019

ghost removed the in progress label Apr 3, 2019

pkalever mentioned this pull request May 15, 2020

gluster-block: add logrotate support #164

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gluster-block: add logrotate support #163

gluster-block: add logrotate support #163

lxbsz commented Nov 19, 2018 •

edited

Loading

pkalever left a comment

pkalever commented Mar 20, 2019

lxbsz commented Mar 20, 2019

lxbsz commented Mar 24, 2019

lxbsz commented Mar 28, 2019

lxbsz commented Mar 29, 2019

pkalever left a comment

lxbsz commented Apr 2, 2019

pkalever commented Apr 2, 2019

pkalever left a comment

pkalever commented Apr 2, 2019

lxbsz commented Apr 3, 2019

lxbsz commented Apr 3, 2019

pkalever left a comment

lxbsz commented Apr 3, 2019

pkalever commented Apr 3, 2019

lxbsz commented Apr 3, 2019

pkalever commented Apr 3, 2019

lxbsz commented Apr 3, 2019

pkalever commented Apr 3, 2019

lxbsz commented Apr 3, 2019 •

edited

Loading

pkalever commented Apr 3, 2019

lxbsz commented Apr 3, 2019

pkalever commented Apr 3, 2019

gluster-block: add logrotate support #163

gluster-block: add logrotate support #163

Conversation

lxbsz commented Nov 19, 2018 • edited Loading

pkalever left a comment

Choose a reason for hiding this comment

pkalever commented Mar 20, 2019

lxbsz commented Mar 20, 2019

lxbsz commented Mar 24, 2019

lxbsz commented Mar 28, 2019

lxbsz commented Mar 29, 2019

pkalever left a comment

Choose a reason for hiding this comment

lxbsz commented Apr 2, 2019

pkalever commented Apr 2, 2019

pkalever left a comment

Choose a reason for hiding this comment

pkalever commented Apr 2, 2019

lxbsz commented Apr 3, 2019

lxbsz commented Apr 3, 2019

pkalever left a comment

Choose a reason for hiding this comment

lxbsz commented Apr 3, 2019

pkalever commented Apr 3, 2019

lxbsz commented Apr 3, 2019

pkalever commented Apr 3, 2019

lxbsz commented Apr 3, 2019

pkalever commented Apr 3, 2019

lxbsz commented Apr 3, 2019 • edited Loading

pkalever commented Apr 3, 2019

lxbsz commented Apr 3, 2019

pkalever commented Apr 3, 2019

lxbsz commented Nov 19, 2018 •

edited

Loading

lxbsz commented Apr 3, 2019 •

edited

Loading