Re: [Gluster-Maintainers] Release 5: New option noatime
On 09/28/2018 03:53 AM, Niels de Vos wrote: > On Fri, Sep 28, 2018 at 10:07:58AM +0530, Amar Tumballi wrote: >> On Thu, Sep 27, 2018 at 6:40 PM Shyam Ranganathan >> wrote: >> >>> On 09/27/2018 09:08 AM, Shyam Ranganathan wrote: Writing this to solicit opinions on merging this [1] change that introduces an option late in the release cycle. I went through the code, and most changes are standard option handling and basic xlator scaffolding, other than the change in posix xlator code that handles the flag to not set atime and the code in utime xlator that conditionally sets the flag. (of which IMO the latter is more important than the former, as posix is just acting on the flag). The option if enabled would hence not update atime for the following FOPs, opendir, open, read, and would continue updating atime on the following FOPs fallocate and zerofill (which also update mtime, so the AFR self heal on time change would kick in anyways). As the option suggests, with it enables atime is almost meaningless and hence it almost does not matter where we update it and where not. Just considering the problem where atime changes cause AFR to trigger a heal, and the FOPs above that strictly only change atime handled with this option, I am looking at this as functionally workable. >>> >> >> Thanks for all these details, Shyam! Helps many to understand what the >> feature is. >> >> So IMO we can accept this even though it is late, but would like to hear from others if this needs to be deferred till release-6. >>> >> >> I am all for accepting this for glusterfs-5.0! Two reasons, in one of the >> quick setup we tried, it helped to get elastic search run smoothly on >> glusterfs mounts. 2nd, we did hear from Anuradha/Ram in another email >> thread (Cloudsync with AFR) that it helped them in solving the issue. >> >> This particular patch makes the overhead of ctime feature much much lesser! > > This seems like a bugfix and not really so much of a feature. From the > description of the patch and in the related BZs I could not find how to > set the option (or is it automatic?). Including it should be fine, but > the documentation/design/spec for the ctime xlator should be ammended as > well. > ... which I could not find in https://github.com/gluster/glusterfs-specs I want to reiterate bug fixes Vs features, and would like to change the latter as enhancements than features. So BZ for bugs, all enhancements are github issues. In this case there is an option (for whatever reason) and the added functionality is an enhancement. As a result I am treating it as such. > > Niels > ___ maintainers mailing list maintainers@gluster.org https://lists.gluster.org/mailman/listinfo/maintainers
Re: [Gluster-Maintainers] Release 5: New option noatime
On Fri, Sep 28, 2018 at 10:07:58AM +0530, Amar Tumballi wrote: > On Thu, Sep 27, 2018 at 6:40 PM Shyam Ranganathan > wrote: > > > On 09/27/2018 09:08 AM, Shyam Ranganathan wrote: > > > Writing this to solicit opinions on merging this [1] change that > > > introduces an option late in the release cycle. > > > > > > I went through the code, and most changes are standard option handling > > > and basic xlator scaffolding, other than the change in posix xlator code > > > that handles the flag to not set atime and the code in utime xlator that > > > conditionally sets the flag. (of which IMO the latter is more important > > > than the former, as posix is just acting on the flag). > > > > > > The option if enabled would hence not update atime for the following > > > FOPs, opendir, open, read, and would continue updating atime on the > > > following FOPs fallocate and zerofill (which also update mtime, so the > > > AFR self heal on time change would kick in anyways). > > > > > > As the option suggests, with it enables atime is almost meaningless and > > > hence it almost does not matter where we update it and where not. Just > > > considering the problem where atime changes cause AFR to trigger a heal, > > > and the FOPs above that strictly only change atime handled with this > > > option, I am looking at this as functionally workable. > > > > > > > Thanks for all these details, Shyam! Helps many to understand what the > feature is. > > > > > So IMO we can accept this even though it is late, but would like to hear > > > from others if this needs to be deferred till release-6. > > > > > > > I am all for accepting this for glusterfs-5.0! Two reasons, in one of the > quick setup we tried, it helped to get elastic search run smoothly on > glusterfs mounts. 2nd, we did hear from Anuradha/Ram in another email > thread (Cloudsync with AFR) that it helped them in solving the issue. > > This particular patch makes the overhead of ctime feature much much lesser! This seems like a bugfix and not really so much of a feature. From the description of the patch and in the related BZs I could not find how to set the option (or is it automatic?). Including it should be fine, but the documentation/design/spec for the ctime xlator should be ammended as well. ... which I could not find in https://github.com/gluster/glusterfs-specs Niels ___ maintainers mailing list maintainers@gluster.org https://lists.gluster.org/mailman/listinfo/maintainers
Re: [Gluster-Maintainers] Release 5: New option noatime
On Thu, Sep 27, 2018 at 6:40 PM Shyam Ranganathan wrote: > On 09/27/2018 09:08 AM, Shyam Ranganathan wrote: > > Writing this to solicit opinions on merging this [1] change that > > introduces an option late in the release cycle. > > > > I went through the code, and most changes are standard option handling > > and basic xlator scaffolding, other than the change in posix xlator code > > that handles the flag to not set atime and the code in utime xlator that > > conditionally sets the flag. (of which IMO the latter is more important > > than the former, as posix is just acting on the flag). > > > > The option if enabled would hence not update atime for the following > > FOPs, opendir, open, read, and would continue updating atime on the > > following FOPs fallocate and zerofill (which also update mtime, so the > > AFR self heal on time change would kick in anyways). > > > > As the option suggests, with it enables atime is almost meaningless and > > hence it almost does not matter where we update it and where not. Just > > considering the problem where atime changes cause AFR to trigger a heal, > > and the FOPs above that strictly only change atime handled with this > > option, I am looking at this as functionally workable. > > > Thanks for all these details, Shyam! Helps many to understand what the feature is. > > So IMO we can accept this even though it is late, but would like to hear > > from others if this needs to be deferred till release-6. > > > I am all for accepting this for glusterfs-5.0! Two reasons, in one of the quick setup we tried, it helped to get elastic search run smoothly on glusterfs mounts. 2nd, we did hear from Anuradha/Ram in another email thread (Cloudsync with AFR) that it helped them in solving the issue. This particular patch makes the overhead of ctime feature much much lesser! -Amar > > Shyam > > [1] Patch under review: https://review.gluster.org/c/glusterfs/+/21281 > ___ > maintainers mailing list > maintainers@gluster.org > https://lists.gluster.org/mailman/listinfo/maintainers > -- Amar Tumballi (amarts) ___ maintainers mailing list maintainers@gluster.org https://lists.gluster.org/mailman/listinfo/maintainers
Re: [Gluster-Maintainers] Release 5: New option noatime
On 09/27/2018 09:08 AM, Shyam Ranganathan wrote: > Writing this to solicit opinions on merging this [1] change that > introduces an option late in the release cycle. > > I went through the code, and most changes are standard option handling > and basic xlator scaffolding, other than the change in posix xlator code > that handles the flag to not set atime and the code in utime xlator that > conditionally sets the flag. (of which IMO the latter is more important > than the former, as posix is just acting on the flag). > > The option if enabled would hence not update atime for the following > FOPs, opendir, open, read, and would continue updating atime on the > following FOPs fallocate and zerofill (which also update mtime, so the > AFR self heal on time change would kick in anyways). > > As the option suggests, with it enables atime is almost meaningless and > hence it almost does not matter where we update it and where not. Just > considering the problem where atime changes cause AFR to trigger a heal, > and the FOPs above that strictly only change atime handled with this > option, I am looking at this as functionally workable. > > So IMO we can accept this even though it is late, but would like to hear > from others if this needs to be deferred till release-6. > > Shyam [1] Patch under review: https://review.gluster.org/c/glusterfs/+/21281 ___ maintainers mailing list maintainers@gluster.org https://lists.gluster.org/mailman/listinfo/maintainers
[Gluster-Maintainers] Release 5: New option noatime
Writing this to solicit opinions on merging this [1] change that introduces an option late in the release cycle. I went through the code, and most changes are standard option handling and basic xlator scaffolding, other than the change in posix xlator code that handles the flag to not set atime and the code in utime xlator that conditionally sets the flag. (of which IMO the latter is more important than the former, as posix is just acting on the flag). The option if enabled would hence not update atime for the following FOPs, opendir, open, read, and would continue updating atime on the following FOPs fallocate and zerofill (which also update mtime, so the AFR self heal on time change would kick in anyways). As the option suggests, with it enables atime is almost meaningless and hence it almost does not matter where we update it and where not. Just considering the problem where atime changes cause AFR to trigger a heal, and the FOPs above that strictly only change atime handled with this option, I am looking at this as functionally workable. So IMO we can accept this even though it is late, but would like to hear from others if this needs to be deferred till release-6. Shyam ___ maintainers mailing list maintainers@gluster.org https://lists.gluster.org/mailman/listinfo/maintainers
