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

Translator to handle 'global' options #303

Closed
amarts opened this issue Aug 16, 2017 · 11 comments
Closed

Translator to handle 'global' options #303

amarts opened this issue Aug 16, 2017 · 11 comments

Comments

@amarts
Copy link
Member

amarts commented Aug 16, 2017

Problem statement

I was checking the glusterfsd/src/glusterfsd.h and noticed that we have more than 50 options to be passed as part of command line.

Trying each of these, and handling these configuration options to be passed to client from config servers (ie, glusterd{1,2}) will be a challenge today. Most of these options are set in 'ctx' (glusterfs_ctx_t) structure itself.

Proposal

I propose having a translator which just has basic required symbols defined and handle parsing the options in init() and then just setting those options in ctx itself. This can be defined at the top of the volfile, by glusterd, so a client behavior can be controlled by server itself.

Benefits:

  • The command line options are static, and one has to restart the process to make the change, with this xlator, we don't have to restart the process.
  • As it is a volfile change, log file captures all the options passed, and hence should be easier to debug later.
  • Many deployment tools like ansible roles / playbooks prefer the config method than the argument approach, so providing the config options in global xlator itself will be better.

add more if you have any?

Some more options which can be passed here?

  • volume file generation number
  • timestamp of the volume file generation ? (or should it be same as generation number?)
  • may be iobuf arena size definition ?
@amarts
Copy link
Member Author

amarts commented Aug 16, 2017

@gluster/gluster-all ☝️ can you guys review this, and suggest more points, Ack/No-ack on it.

@kshlm
Copy link
Member

kshlm commented Aug 16, 2017

I'd prefer another name for this, possibly daemon options or something. Global options should be used to refer to something that affects things all over the cluster.

@amarts
Copy link
Member Author

amarts commented Aug 16, 2017

@kshlm how about meta/process-option-parser ?

@csabahenk
Copy link
Member

While it's hard to not agree with the benefits of more dynamic runtime behavior, the argument for easier debuggability:

As it is a volfile change, log file captures all the options passed, and hence should be easier to debug later.

is moot. The command line options with which the a glusterfs process is started is already recorded in the log. On the other hand, it will be confusing to track down which part of the current behavior is originating from server and which from client.

Related concern: what would be the precedence of server and client (command line) options? What sounds most sane to me is that server makes suggestions but client can can decide, ie. client takes precedence. But that's somewhat leaky abstraction, because then when at runtime glusterd sends a new volfile with new options, it will eventually override clientside preferences. Whatever choice is made, the proposal should declare precedence.

This could be amended by having two process option xlators, one being served, other encapsulating the option set of the client command line. That also could be a means to cleanly indicate the situation with runtime updates and precedence:

  • if we settle with a semantics that client command line options are preserved throughout the client's lifetime, then we can just keep and insert the client command line option xlator upon any runtime volfile update.
  • If we allow the server to override any client option via a runtime update, then we can just drop the client command line option xlator when the volfile is updated.

Another UI concern: while the disctinction between those command line opts which also manifest as options of the meta-xlator and those who are solely command line -- first group corresponding to fields of ctx -- is technically clear, it might be not so clear from the user's POV. Is there any idea how to help them to distinguish? If nothing else, a well defined and executed documentation policy regarding options would be a desired augmentation of the proposal.

@amarts
Copy link
Member Author

amarts commented Aug 17, 2017

@csabahenk Thanks for going through the proposal! I agree on most of the points here. This is surely up for much larger discussion.

One of the thing I was thinking is, any command line argument given to binary should take precedence over any global options, and shouldn't change even if server/config changes. The 'process-option-parser' should consider changing the value of fields which are left to default, and not given in CLI.

With that handling, we can say the behavior of client side process is managed by our servers (glusterd/glusterd2), but one can override with with CLI args.

Let me know what you think.

@jdarcy
Copy link
Contributor

jdarcy commented Aug 17, 2017

Having a whole translator just to hold options seems a bit wasteful. I'd much rather put them on global_xlator, which already exists but is not part of the I/O path. There are a couple of ways we could do this.

(1) Pretend there's a separate translator for purposes of volfiles, info files, command-line parsing, etc. but actually redirect references to global_xlator instead of creating and linking a new one.

(2) Extend the volfile syntax to support options outside of translators.

The graph-comparison and graph-switch code will have to be tweaked a bit either way, but that's probably true on the server side anyway because of multiplexing so I don't think it's a big deal.

@amarts
Copy link
Member Author

amarts commented Sep 1, 2017

@jdarcy I am more inclined on (1) above, and it makes sense to not have new xlator for this.

Any comments on @csabahenk 's concerns? I guess making command line override all the options is a better path.

@gluster-ant
Copy link
Collaborator

A patch https://review.gluster.org/18315 has been posted that references this issue.
Commit message: Add framework for global xlator in graph

1 similar comment
@gluster-ant
Copy link
Collaborator

A patch https://review.gluster.org/18315 has been posted that references this issue.
Commit message: Add framework for global xlator in graph

@gluster-ant
Copy link
Collaborator

A patch https://review.gluster.org/18316 has been posted that references this issue.
Commit message: global options: add a sample option to handle

mscherer pushed a commit that referenced this issue Nov 3, 2017
Updates #303

Change-Id: Id0b9050c93ea87532dc80b4fda650c5663d285bd
Signed-off-by: Amar Tumballi <amarts@redhat.com>
@gluster-ant
Copy link
Collaborator

A patch https://review.gluster.org/18316 has been posted that references this issue.
Commit message: global options: add a sample option to handle

@ShyamsundarR ShyamsundarR added this to the Release 4.0 (STM) milestone Jan 16, 2018
@ShyamsundarR ShyamsundarR added this to Release 4.0 in Releases Jan 16, 2018
amarts added a commit to amarts/glusterfs_fork that referenced this issue Sep 11, 2018
Updates gluster#303

Change-Id: Id0b9050c93ea87532dc80b4fda650c5663d285bd
Signed-off-by: Amar Tumballi <amarts@redhat.com>
amarts added a commit to amarts/glusterfs_fork that referenced this issue Sep 11, 2018
Fixes gluster#303

Change-Id: Icdaa804711c43c65b9684f2649437aae1b5c1ed5
Signed-off-by: Amar Tumballi <amarts@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants