gluster-containers icon indicating copy to clipboard operation
gluster-containers copied to clipboard

variable for sleep seconds using systemd environment.

Open SaravanaStorageNetwork opened this issue 7 years ago • 12 comments

Signed-off-by: Saravanakumar Arumugam [email protected]

SaravanaStorageNetwork avatar Aug 29 '18 11:08 SaravanaStorageNetwork

@phlogistonjohn @humblec @raghavendra-talur @jarrpa PTAL

SaravanaStorageNetwork avatar Aug 29 '18 11:08 SaravanaStorageNetwork

@SaravanaStorageNetwork LGTM. Waiting for one more lgtm before merging :)

humblec avatar Aug 29 '18 11:08 humblec

This doesn't seem to solve the problem at all. Best I can tell from the Environment directive, this does not merely set a default value for the env var but actually sets the value for the script's environment. I think John's initial suggestion of using PassEnvironment is correct, and that you should modify the script to have something like POLL_INTERVAL=${POLL_INTERVAL:-120} at the beginning. Also, the name shoud be more descriptive, maybe something like DISK_CHECK_POLL_INTERVAL.

jarrpa avatar Aug 29 '18 14:08 jarrpa

I agree with everything @jarrpa said in his previous comment.

phlogistonjohn avatar Aug 29 '18 18:08 phlogistonjohn

I just remembered to add here: Also the sleep should probably happen at the end of the loop. Unless there's a reason we want to wait before checking if the disk space is full.

jarrpa avatar Aug 29 '18 18:08 jarrpa

I think John's initial suggestion of using PassEnvironment is correct, and that you should modify the script to have something like POLL_INTERVAL=${POLL_INTERVAL:-120} at the beginning.

Oops - I have missed the comment related to PassEnvironment by @phlogistonjohn - will check this out and update.

the sleep should probably happen at the end of the loop. Unless there's a reason we want to wait before checking if the disk space is full.

Reason to have the early wait is for glusterd to be up and running. As you can see from the systemd file , this service is started after glusterd service (after all it is checking the configuration directory created/managed by glusterd).

Glusterd takes time to start(especially if too many volumes configured) and hence the wait immediate after glusterd start and then doing the check..

SaravanaStorageNetwork avatar Aug 30 '18 11:08 SaravanaStorageNetwork

I don't understand why we're waiting for glusterd. We're just checking disk space, what purpose does waiting for glusterd serve?

jarrpa avatar Aug 30 '18 16:08 jarrpa

I don't understand why we're waiting for glusterd. We're just checking disk space, what purpose does waiting for glusterd serve?

Running out of space is ( very )worst case scenario , not required to be checked even before glusterd getting settled. glusterd daemon may need to connect to other peers and it may update content inside /var/lib/glusterd. This is just to give some room for glusterd daemon.

SaravanaStorageNetwork avatar Aug 31 '18 01:08 SaravanaStorageNetwork

It's not REQUIRED, okay, but is there an actual technical reason as to why you SHOULD or MUST wait? I still see no relation between glusterd being ready and checking for disk space, and thus no reason to wait.

Regardless, if no one else cares I'll just leave it there. The more important point is the PassEnvironment directive.

jarrpa avatar Aug 31 '18 01:08 jarrpa

@SaravanaStorageNetwork ping

humblec avatar Oct 08 '18 07:10 humblec

@phlogistonjohn @humblec PTAL

SaravanaStorageNetwork avatar Nov 05 '18 14:11 SaravanaStorageNetwork

+1 on PassEnvironment

As @obnoxxx mentioned in a different context we may want to do something about some of the duplication between the this internal server and the external facing probe script, but we can tackle that separately.

phlogistonjohn avatar Nov 05 '18 16:11 phlogistonjohn