Re: [Gluster-Maintainers] Release 5: New option noatime

2018-09-28 Thread Shyam Ranganathan
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

2018-09-28 Thread Niels de Vos
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

2018-09-27 Thread Amar Tumballi
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

2018-09-27 Thread Shyam Ranganathan
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

2018-09-27 Thread Shyam Ranganathan
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