[Gluster-devel] tests/basic/pump.t - what is it used for?

[Atin Mukherjee]

Pranith,

I see you're the author of the test in $Subj. Now while I was working on a
patch https://review.gluster.org/#/c/18226/ to disallow replace brick
operations on dist only volumes the patch failed the regression on this
test as the test actually uses replace brick on a distribute only volume
which IMO is wrong as then this would always end up in to data loss
situation. I'd need some context here to understand the expectation of this
test before doing any modifications.

~Atin
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel

Re: [Gluster-devel] Need inputs on patch #17985

[Raghavendra Gowdappa]

- Original Message -
> From: "FNU Raghavendra Manjunath" 
> To: "Raghavendra Gowdappa" 
> Cc: "Raghavendra G" , "Nithya Balachandran" 
> , anoo...@redhat.com,
> "Gluster Devel" , "Raghavendra Bhat" 
> 
> Sent: Thursday, September 7, 2017 6:44:51 PM
> Subject: Re: [Gluster-devel] Need inputs on patch #17985
> 
> From snapview client perspective one important thing to note. For building
> the context for the entry point (by default ".snaps") a explicit lookup has
> to be done on it. The dentry for ".snaps" is not returned when readdir is
> done on its parent directory (Not even when ls -a is done). So for building
> the context of .snaps (in the context snapview client saves the information
> about whether it is a real inode or virtual inode) we need a lookup.

Since the dentry corresponding to ".snaps" is not returned, there won't be an 
inode for this directory linked in itable. Also, glusterfs wouldn't have given 
nodeid corresponding to ".snaps" during readdir response (as dentry itself is 
not returned). So, kernel would do an explicit lookup before doing any 
operation on ".snaps" (unlike for those dentries which contain nodeid kernel 
can choose to skip a lookup) and we are safe. So, #17985 is safe in its current 
form.

> 
> From snapview server perspective as well a lookup might be needed. In
> snapview server a glfs handle is established between the snapview server
> and the snapshot brick. So a inode in snapview server process contains the
> glfs handle for the object being accessed from snapshot.  In snapview
> server readdirp does not build the inode context (which contains the glfs
> handle etc) because glfs handle is returned only in lookup.

Same argument I've given holds good for this case too. Important point to note 
is that "there is no dentry and hence no nodeid corresponding to .snaps is 
passed to kernel and kernel is forced to do an explicit lookup".

> 
> Regards,
> Raghavendra
> 
> 
> On Tue, Aug 29, 2017 at 12:53 AM, Raghavendra Gowdappa 
> wrote:
> 
> >
> >
> > - Original Message -
> > > From: "Raghavendra G" 
> > > To: "Nithya Balachandran" 
> > > Cc: "Raghavendra Gowdappa" , anoo...@redhat.com,
> > "Gluster Devel" ,
> > > raghaven...@redhat.com
> > > Sent: Tuesday, August 29, 2017 8:52:28 AM
> > > Subject: Re: [Gluster-devel] Need inputs on patch #17985
> > >
> > > On Thu, Aug 24, 2017 at 2:53 PM, Nithya Balachandran <
> > nbala...@redhat.com>
> > > wrote:
> > >
> > > > It has been a while but iirc snapview client (loaded abt dht/tier etc)
> > had
> > > > some issues when we ran tiering tests. Rafi might have more info on
> > this -
> > > > basically it was expecting to find the inode_ctx populated but it was
> > not.
> > > >
> > >
> > > Thanks Nithya. @Rafi, @Raghavendra Bhat, is it possible to take the
> > > ownership of,
> > >
> > > * Identifying whether the patch in question causes the issue?
> >
> > gf_svc_readdirp_cbk is setting relevant state in inode [1]. I quickly
> > checked whether its the same state stored by gf_svc_lookup_cbk and it looks
> > like the same state. So, I guess readdirp is handled correctly by
> > snapview-client and an explicit lookup is not required. But, will wait for
> > inputs from rabhat and rafi.
> >
> > [1] https://github.com/gluster/glusterfs/blob/master/xlators/
> > features/snapview-client/src/snapview-client.c#L1962
> >
> > > * Send a fix or at least evaluate whether a fix is possible.
> > >
> > > @Others,
> > >
> > > With the motivation of getting some traction on this, Is it ok if we:
> > > * Set a deadline of around 15 days to complete the review (or testing
> > with
> > > the patch in question) of respective components and to come up with
> > issues
> > > (if any).
> > > * Post the deadline, if there are no open issues, go ahead and merge the
> > > patch?
> > >
> > > If time is not enough, let us know and we can come up with a reasonable
> > > time.
> > >
> > > regards,
> > > Raghavendra
> > >
> > >
> > > > On 24 August 2017 at 10:13, Raghavendra G 
> > > > wrote:
> > > >
> > > >> Note that we need to consider xlators on brick stack too. I've added
> > > >> maintainers/peers of xlators on brick stack. Please explicitly
> > ack/nack
> > > >> whether this patch affects your component.
> > > >>
> > > >> For reference, following are the xlators loaded in brick stack
> > > >>
> > > >> storage/posix
> > > >> features/trash
> > > >> features/changetimerecorder
> > > >> features/changelog
> > > >> features/bitrot-stub
> > > >> features/access-control
> > > >> features/locks
> > > >> features/worm
> > > >> features/read-only
> > > >> features/leases
> > > >> features/upcall
> > > >> performance/io-threads
> > > >> features/selinux
> > > >> 

Re: [Gluster-devel] Fuse mounts and inodes

[Csaba Henk]

On Wed, Sep 6, 2017 at 4:49 PM, Raghavendra G 
wrote:

>
>
> On Wed, Sep 6, 2017 at 11:16 AM, Csaba Henk  wrote:
>
>> Thanks Du, nice bit of info! It made me wander about the following:
>>
>> - Could it be then the default answer we give to "glusterfs client
>> high memory usage"
>>   type of complaints to set vfs_cache_pressure to 100 + x?
>>
> - And then x = ? Was there proper performance testing done to see how
>> performance /
>>   mem consumtion changes in terms of vfs_cache_performace?
>>
>
> I had a discussion with Manoj on this. One drawback with using
> vfs_cache_performance tunable is that its a dynamic algorithm which decides
> whether to purge from page cache or inode cache looking at the current
> memory pressure. An obvious drawback for glusterfs is that, various of
> caches of glusterfs are not visible to kernel (Memory consumed by Glusterfs
> gets reflected neither in page cache nor in inode cache). This _might_
> result in algorithm working poorly.
>

It does not seem to be a big concern. vfs_cache_pressure was brought up in
context of the inode proliferation issue, and (unlike "various caches") the
size of the gluster client's inode herd is duely reflected in kernel
because of the one to one mapping of the inodes of the two (which is the
very cause of the phenomenon).

Csaba



> - vfs_cache_pressure is an allover system tunable. If 100 + x is ideal
>> for GlusterFS, can
>>   we take the courage to propose this? Is there no risk to trash other
>> (disk-based)
>>   filesystems' performace?
>>
>
> That's a valid point. Behavior of other filesystems would be a concern.
>
> I've not really thought through this suggestion of tuning /proc/sys/vm
> tunables and I am not even an expert who knows what tunables are at our
> disposal. Just wanted to bring this idea to notice of wider audience.
>
>
>> Csaba
>>
>> On Wed, Sep 6, 2017 at 6:57 AM, Raghavendra G 
>> wrote:
>> > Another parallel effort could be trying to configure the number of
>> > inodes/dentries cached by kernel VFS using /proc/sys/vm interface.
>> >
>> > ==
>> >
>> > vfs_cache_pressure
>> > --
>> >
>> > This percentage value controls the tendency of the kernel to reclaim
>> > the memory which is used for caching of directory and inode objects.
>> >
>> > At the default value of vfs_cache_pressure=100 the kernel will attempt
>> to
>> > reclaim dentries and inodes at a "fair" rate with respect to pagecache
>> and
>> > swapcache reclaim.  Decreasing vfs_cache_pressure causes the kernel to
>> > prefer
>> > to retain dentry and inode caches. When vfs_cache_pressure=0, the kernel
>> > will
>> > never reclaim dentries and inodes due to memory pressure and this can
>> easily
>> > lead to out-of-memory conditions. Increasing vfs_cache_pressure beyond
>> 100
>> > causes the kernel to prefer to reclaim dentries and inodes.
>> >
>> > Increasing vfs_cache_pressure significantly beyond 100 may have negative
>> > performance impact. Reclaim code needs to take various locks to find
>> > freeable
>> > directory and inode objects. With vfs_cache_pressure=1000, it will look
>> for
>> > ten times more freeable objects than there are.
>> >
>> > Also we've an article for sysadmins which has a section:
>> >
>> > 
>> >
>> > With GlusterFS, many users with a lot of storage and many small files
>> > easily end up using a lot of RAM on the server side due to
>> > 'inode/dentry' caching, leading to decreased performance when the kernel
>> > keeps crawling through data-structures on a 40GB RAM system. Changing
>> > this value higher than 100 has helped many users to achieve fair caching
>> > and more responsiveness from the kernel.
>> >
>> > 
>> >
>> > Complete article can be found at:
>> > https://gluster.readthedocs.io/en/latest/Administrator%20Gui
>> de/Linux%20Kernel%20Tuning/
>> >
>> > regards,
>> >
>> >
>> > On Tue, Sep 5, 2017 at 5:20 PM, Raghavendra Gowdappa <
>> rgowd...@redhat.com>
>> > wrote:
>> >>
>> >> +gluster-devel
>> >>
>> >> Ashish just spoke to me about need of GC of inodes due to some state in
>> >> inode that is being proposed in EC. Hence adding more people to
>> >> conversation.
>> >>
>> >> > > On 4 September 2017 at 12:34, Csaba Henk  wrote:
>> >> > >
>> >> > > > I don't know, depends on how sophisticated GC we need/want/can
>> get
>> >> > > > by. I
>> >> > > > guess the complexity will be inherent, ie. that of the algorithm
>> >> > > > chosen
>> >> > > > and
>> >> > > > how we address concurrency & performance impacts, but once that's
>> >> > > > got
>> >> > > > right
>> >> > > > the other aspects of implementation won't be hard.
>> >> > > >
>> >> > > > Eg. would it be good just to maintain a simple LRU list?
>> >> > > >
>> >> >
>> >> > Yes. I was also thinking of leveraging lru list. We can invalidate
>> first
>> >> > "n"
>> >> > inodes from lru list of fuse inode table.
>> >> >

Re: [Gluster-devel] Need inputs on patch #17985

[FNU Raghavendra Manjunath]

>From snapview client perspective one important thing to note. For building
the context for the entry point (by default ".snaps") a explicit lookup has
to be done on it. The dentry for ".snaps" is not returned when readdir is
done on its parent directory (Not even when ls -a is done). So for building
the context of .snaps (in the context snapview client saves the information
about whether it is a real inode or virtual inode) we need a lookup.

>From snapview server perspective as well a lookup might be needed. In
snapview server a glfs handle is established between the snapview server
and the snapshot brick. So a inode in snapview server process contains the
glfs handle for the object being accessed from snapshot.  In snapview
server readdirp does not build the inode context (which contains the glfs
handle etc) because glfs handle is returned only in lookup.

Regards,
Raghavendra


On Tue, Aug 29, 2017 at 12:53 AM, Raghavendra Gowdappa 
wrote:

>
>
> - Original Message -
> > From: "Raghavendra G" 
> > To: "Nithya Balachandran" 
> > Cc: "Raghavendra Gowdappa" , anoo...@redhat.com,
> "Gluster Devel" ,
> > raghaven...@redhat.com
> > Sent: Tuesday, August 29, 2017 8:52:28 AM
> > Subject: Re: [Gluster-devel] Need inputs on patch #17985
> >
> > On Thu, Aug 24, 2017 at 2:53 PM, Nithya Balachandran <
> nbala...@redhat.com>
> > wrote:
> >
> > > It has been a while but iirc snapview client (loaded abt dht/tier etc)
> had
> > > some issues when we ran tiering tests. Rafi might have more info on
> this -
> > > basically it was expecting to find the inode_ctx populated but it was
> not.
> > >
> >
> > Thanks Nithya. @Rafi, @Raghavendra Bhat, is it possible to take the
> > ownership of,
> >
> > * Identifying whether the patch in question causes the issue?
>
> gf_svc_readdirp_cbk is setting relevant state in inode [1]. I quickly
> checked whether its the same state stored by gf_svc_lookup_cbk and it looks
> like the same state. So, I guess readdirp is handled correctly by
> snapview-client and an explicit lookup is not required. But, will wait for
> inputs from rabhat and rafi.
>
> [1] https://github.com/gluster/glusterfs/blob/master/xlators/
> features/snapview-client/src/snapview-client.c#L1962
>
> > * Send a fix or at least evaluate whether a fix is possible.
> >
> > @Others,
> >
> > With the motivation of getting some traction on this, Is it ok if we:
> > * Set a deadline of around 15 days to complete the review (or testing
> with
> > the patch in question) of respective components and to come up with
> issues
> > (if any).
> > * Post the deadline, if there are no open issues, go ahead and merge the
> > patch?
> >
> > If time is not enough, let us know and we can come up with a reasonable
> > time.
> >
> > regards,
> > Raghavendra
> >
> >
> > > On 24 August 2017 at 10:13, Raghavendra G 
> > > wrote:
> > >
> > >> Note that we need to consider xlators on brick stack too. I've added
> > >> maintainers/peers of xlators on brick stack. Please explicitly
> ack/nack
> > >> whether this patch affects your component.
> > >>
> > >> For reference, following are the xlators loaded in brick stack
> > >>
> > >> storage/posix
> > >> features/trash
> > >> features/changetimerecorder
> > >> features/changelog
> > >> features/bitrot-stub
> > >> features/access-control
> > >> features/locks
> > >> features/worm
> > >> features/read-only
> > >> features/leases
> > >> features/upcall
> > >> performance/io-threads
> > >> features/selinux
> > >> features/marker
> > >> features/barrier
> > >> features/index
> > >> features/quota
> > >> debug/io-stats
> > >> performance/decompounder
> > >> protocol/server
> > >>
> > >>
> > >> For those not following this thread, the question we need to answer
> is,
> > >> "whether the xlator you are associated with works fine if a non-lookup
> > >> fop (like open, setattr, stat etc) hits it without a lookup ever being
> > >> done
> > >> on that inode"
> > >>
> > >> regards,
> > >> Raghavendra
> > >>
> > >> On Wed, Aug 23, 2017 at 11:56 AM, Raghavendra Gowdappa <
> > >> rgowd...@redhat.com> wrote:
> > >>
> > >>> Thanks Pranith and Ashish for your inputs.
> > >>>
> > >>> - Original Message -
> > >>> > From: "Pranith Kumar Karampuri" 
> > >>> > To: "Ashish Pandey" 
> > >>> > Cc: "Raghavendra Gowdappa" , "Xavier
> Hernandez" <
> > >>> xhernan...@datalab.es>, "Gluster Devel"
> > >>> > 
> > >>> > Sent: Wednesday, August 23, 2017 11:55:19 AM
> > >>> > Subject: Re: Need inputs on patch #17985
> > >>> >
> > >>> > Raghavendra,
> > >>> > As Ashish mentioned, there aren't any known problems if upper
> > >>> xlators
> > >>> > don't send lookups in EC at the moment.
> > >>> >
> > >>> > On Wed, Aug 23, 2017 at 9:07 AM, Ashish Pandey <
> aspan...@redhat.com>
> > >>> wrote:
> > 

Re: [Gluster-devel] [Gluster-Maintainers] Changing Submit Type on review.gluster.org

[Niels de Vos]

On Thu, Sep 07, 2017 at 04:11:22PM +0530, Milind Changire wrote:
> *Squashed Patches*
> I believe, individual engineers have to own the responsibility of
> maintaining history of all appropriate Change-Ids as part of the commit
> message when multiple patches have been squashed/merged into one commit.

We do not recommend squashing multiple patches into one at any case. It
is much easier to follow the modifications when each functional change
has its own patch. This counts for the master branch, but also for
backports.

Niels


> 
> 
> 
> 
> On Thu, Sep 7, 2017 at 11:50 AM, Nigel Babu  wrote:
> 
> > Hello folks,
> >
> > A few times, we've merged dependent patches out of order because the Submit
> > type[1] did not block us from doing so. The last few times we've talked
> > about
> > this, we didn't actually take a strong decision either way. In yesterday's
> > maintainers meeting, we agreed to change the Submit type to
> > Rebase-If-Necessary. This change will happen on 18th September 2017.
> >
> > What this means:
> > * No more metadata flags added by Gerrit. There will only be a Change-Id,
> >   Signed-off-by, and BUG (if you've added it). Gerrit itself will not add
> > any
> >   metadata.
> > * If you push a patch on top of another patch, the Submit button will
> > either be
> >   grayed out because the dependent patches cannot be merged or they will be
> >   submited in the correct order in one go.
> >
> > Some of the concerns that have been raised:
> > Q: With the Reviewed-on flag gone, how do we keep track of changesets
> >(especially backports)?
> > A: The Change-Id will get you all the data directly on Gerrit. As long you
> >retain the Change-Id, Gerrit will get you the matching changesets.
> >
> > Q: Will who-wrote-what continue to work?
> > A: As far as I can see, it continues to work. I ran the script against
> >build-jobs repo and it works correctly. Additionally, we'll be setting
> > up an
> >instance of Gerrit Stats[2] to provide more detailed stats.
> >
> > Q: Can we have some of the metadata if not all?
> > Q: Why can't we have the metadata if we change the submit type?
> > A: There's no good answer to this other than, this is how Gerrit works and
> >I can neither change it nor control it.
> >
> > [1]: https://review.gluster.org/Documentation/intro-project-
> > owner.html#submit-type
> > [2]: http://gerritstats-demo.firebaseapp.com/
> >
> > --
> > nigelb
> > ___
> > maintainers mailing list
> > maintain...@gluster.org
> > http://lists.gluster.org/mailman/listinfo/maintainers
> >
> 
> 
> 
> -- 
> Milind

> ___
> maintainers mailing list
> maintain...@gluster.org
> http://lists.gluster.org/mailman/listinfo/maintainers

___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel

Re: [Gluster-devel] [Gluster-Maintainers] Changing Submit Type on review.gluster.org

[Kaushal M]

On 7 Sep 2017 6:25 pm, "Niels de Vos"  wrote:

On Thu, Sep 07, 2017 at 04:41:54PM +0530, Nigel Babu wrote:
> On Thu, Sep 07, 2017 at 12:43:28PM +0200, Niels de Vos wrote:
> >
> > Q: Can patches of a series be merged before all patches in the series
> > have a +2? Initial changes that prepare things, or add new (unused) core
> > functionalities should be mergable so that follow-up patches can be
> > posted against the HEAD of the branch.
> >
> > A: Nigel?
> >
>
> If you have patches that are dependent like this:
>
> A -> B -> C -> D
>
> where A is the first patch and B is based on top of A and so forth.
>
> Merging A is not dependent on B. It can be merged any time you have Code
Review
> and Regression votes.
>
> However, you cannot merge B until A is ready or merged. If A is unmerged,
but
> is ready to merge, when you merge B, Gerrit will merge them in order, i.e.
> first merge A, and B automatically.
>
> Does this answer your question? If it helps, I can arrange for staging to
be
> online so moe people can test this out.

That answers my question, I don't need to try it out myself.

Thanks!
Niels
___
maintainers mailing list
maintain...@gluster.org
http://lists.gluster.org/mailman/listinfo/maintainers



Gerrit still provides all the meta information about patches in a special
branch as git-notes. Git can be configured to display these notes along
with commit messages. You would still effectively get the same experience
as before.

More information is available at [1]. This depends on a gerrit plugin, but
I believe it's enabled by default.

[1]
https://gerrit.googlesource.com/plugins/reviewnotes/+/master/src/main/resources/Documentation/refs-notes-review.md
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel

Re: [Gluster-devel] Changing Submit Type on review.gluster.org

[Niels de Vos]

On Thu, Sep 07, 2017 at 04:41:54PM +0530, Nigel Babu wrote:
> On Thu, Sep 07, 2017 at 12:43:28PM +0200, Niels de Vos wrote:
> >
> > Q: Can patches of a series be merged before all patches in the series
> > have a +2? Initial changes that prepare things, or add new (unused) core
> > functionalities should be mergable so that follow-up patches can be
> > posted against the HEAD of the branch.
> >
> > A: Nigel?
> >
> 
> If you have patches that are dependent like this:
> 
> A -> B -> C -> D
> 
> where A is the first patch and B is based on top of A and so forth.
> 
> Merging A is not dependent on B. It can be merged any time you have Code 
> Review
> and Regression votes.
> 
> However, you cannot merge B until A is ready or merged. If A is unmerged, but
> is ready to merge, when you merge B, Gerrit will merge them in order, i.e.
> first merge A, and B automatically.
> 
> Does this answer your question? If it helps, I can arrange for staging to be
> online so moe people can test this out.

That answers my question, I don't need to try it out myself.

Thanks!
Niels
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel

[Gluster-devel] Coverity covscan for 2017-09-07-eb2f1ab4 (master branch)

[staticanalysis]

GlusterFS Coverity covscan results are available from
http://download.gluster.org/pub/gluster/glusterfs/static-analysis/master/glusterfs-coverity/2017-09-07-eb2f1ab4
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel

Re: [Gluster-devel] [Gluster-Maintainers] Changing Submit Type on review.gluster.org

[Shyam Ranganathan]

On 09/07/2017 02:20 AM, Nigel Babu wrote:

Hello folks,

A few times, we've merged dependent patches out of order because the Submit
type[1] did not block us from doing so. The last few times we've talked about
this, we didn't actually take a strong decision either way. In yesterday's
maintainers meeting, we agreed to change the Submit type to
Rebase-If-Necessary. This change will happen on 18th September 2017.


Unrelated: We possibly need to document on how to work with dependent 
patches, i.e how to create them? manage intermediate changes, etc.


I state this as it is a useful manner of keeping commits small and 
relevant and easy to review, and building on that practice with better 
instructions can help.


On topic: this would be a welcome change than breaking master when such 
events happen, or chasing dependencies (determining which patch is 
dependent on which) when merging the same.




What this means:
* No more metadata flags added by Gerrit. There will only be a Change-Id,
   Signed-off-by, and BUG (if you've added it). Gerrit itself will not add any
   metadata.
* If you push a patch on top of another patch, the Submit button will either be
   grayed out because the dependent patches cannot be merged or they will be
   submited in the correct order in one go.

Some of the concerns that have been raised:
Q: With the Reviewed-on flag gone, how do we keep track of changesets
(especially backports)?
A: The Change-Id will get you all the data directly on Gerrit. As long you
retain the Change-Id, Gerrit will get you the matching changesets.

Q: Will who-wrote-what continue to work?
A: As far as I can see, it continues to work. I ran the script against
build-jobs repo and it works correctly. Additionally, we'll be setting up an
instance of Gerrit Stats[2] to provide more detailed stats.

Q: Can we have some of the metadata if not all?
Q: Why can't we have the metadata if we change the submit type?
A: There's no good answer to this other than, this is how Gerrit works and
I can neither change it nor control it.

[1]: 
https://review.gluster.org/Documentation/intro-project-owner.html#submit-type
[2]: http://gerritstats-demo.firebaseapp.com/

--
nigelb
___
maintainers mailing list
maintain...@gluster.org
http://lists.gluster.org/mailman/listinfo/maintainers


___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel

Re: [Gluster-devel] Build on FreeBSD and gcc version

[Michael Scherer]

Le jeudi 07 septembre 2017 à 11:17 +0200, Niels de Vos a écrit :
> On Wed, Sep 06, 2017 at 12:39:42PM +0200, Michael Scherer wrote:
> > Hi,
> > 
> > so I have been trying to make the internal freebsd builder usable,
> > sinc
> > eit was freshly installed and not building anything.
> > 
> > Over the course of the day, I found a few missing deps, found a ton
> > of
> > warnings (I started to fix the easy one, not sure what to do for
> > the
> > more complicated one around gf_log formating), but yesterday, I
> > stumbled on a more puzzling issue:
> > 
> >  CCLD glusterfsd
> > ../../contrib/argp-standalone/libargp.a(argp-help.o): In function
> > `_help':
> > /tmp/glusterfs/contrib/argp-standalone/argp-help.c:1608: undefined
> > reference to `argp_fmtstream_set_wmargin'
> > /tmp/glusterfs/contrib/argp-standalone/argp-help.c:1622: undefined
> > reference to `argp_fmtstream_set_lmargin'
> 
> FreeBSD seems to provide argp-standalone as a port. It would be
> better
> to use that instead of our bundled version.
>   https://www.freebsd.org/ports/devel.html#argp-standalone-1.3_2

Yeah, I tried, it wasn't picked up.
And I am not keen on digging in configure.ac to make it work :)

Let's see if my last change on build.sh are fixing it:
https://github.com/gluster/glusterfs-patch-acceptance-tests/pull/89

> 
> > I also see message like this:
> > libtool: warning: relinking 'libgfapi.la'
> > *** Warning: Linking the shared library libgfapi.la against the
> > *** static library ../../contrib/argp-standalone/libargp.a is not
> > portable!
> 
> libgfapi should not need to link with libargp. This probably is an
> error
> in LDFLAGS or something.
>  
> > After looking at the old builder, I found a few differences:
> > - old one is FreeBSD 10.1, new one is 10.3
> > - old one is using gcc 4.8, new one was on clang.
> > 
> > So I did install gcc, turn out that this was the version 5 by
> > default
> > on FreeBSD and it still do not work with it.
> > 
> > Then I forced gcc4.8 and now it build fine.
> > Even better, once I forced the version, it also build (but I
> > suspect
> > that's just some bad cleanup on my side, more tests are needed)
> > 
> > Any reason on why this would fail on gcc 5, and not 4.8, and what
> > we
> > should do for that ?
> 
> I don't know of any reason, but we should fix it somehow.

Yup, but si, it seems it wasn't related to gcc version exactly.

But this also bring the question of what should we use on freebsd for
testing, since default is clang, and there is 5 versions of gcc. 

We might also need to clean flags passed to the compiler, since for
example -rdynamic is not supported by clang (even if I discovered that
flag yesterday)

And another question is wether we should push -Werror as well on that
platform, since there is a certain amount of warnings that would be
hard to fix (most notably the whole issue of pthread_t being a opaque
type in POSIX, and that we try to print somehow) both on gcc and clang.

But they are technically bugs (and a standard violation too, even if it
seems the standard is missing a proper function for that) 
 

> > I also found that what fail is to build only in Jenkins, most
> > likely
> > due to the setup where we build with a out of source tree directory
> > for
> > binary. It work fine if doing ./configure ; make ; make install  
> 
> Yeah, this is one of the things that people want to see supported
> (although I don't remember the reason). It breaks every now and then
> :-/

It shouldn't, since we test it. But as my 2nd mail show, my debugging
lead me to wrong conclusions.

> HTH,
> Niels
-- 
Michael Scherer
Sysadmin, Community Infrastructure and Platform, OSAS



signature.asc
Description: This is a digitally signed message part
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel

Re: [Gluster-devel] Changing Submit Type on review.gluster.org

[Nigel Babu]

On Thu, Sep 07, 2017 at 12:43:28PM +0200, Niels de Vos wrote:
>
> Q: Can patches of a series be merged before all patches in the series
> have a +2? Initial changes that prepare things, or add new (unused) core
> functionalities should be mergable so that follow-up patches can be
> posted against the HEAD of the branch.
>
> A: Nigel?
>

If you have patches that are dependent like this:

A -> B -> C -> D

where A is the first patch and B is based on top of A and so forth.

Merging A is not dependent on B. It can be merged any time you have Code Review
and Regression votes.

However, you cannot merge B until A is ready or merged. If A is unmerged, but
is ready to merge, when you merge B, Gerrit will merge them in order, i.e.
first merge A, and B automatically.

Does this answer your question? If it helps, I can arrange for staging to be
online so moe people can test this out.

--
nigelb
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel

Re: [Gluster-devel] [Gluster-Maintainers] Changing Submit Type on review.gluster.org

[Nigel Babu]

On Thu, Sep 07, 2017 at 04:11:22PM +0530, Milind Changire wrote:
> *Squashed Patches*
> I believe, individual engineers have to own the responsibility of
> maintaining history of all appropriate Change-Ids as part of the commit
> message when multiple patches have been squashed/merged into one commit.

This is accurate. We'll need to continue to maintain this as we currently do
for backports.

--
nigelb
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel

Re: [Gluster-devel] [Gluster-Maintainers] Changing Submit Type on review.gluster.org

[Niels de Vos]

On Thu, Sep 07, 2017 at 12:06:19PM +0530, Amar Tumballi wrote:
> On Thu, Sep 7, 2017 at 11:50 AM, Nigel Babu  wrote:
> 
> > Hello folks,
> >
> > A few times, we've merged dependent patches out of order because the Submit
> > type[1] did not block us from doing so. The last few times we've talked
> > about
> > this, we didn't actually take a strong decision either way. In yesterday's
> > maintainers meeting, we agreed to change the Submit type to
> > Rebase-If-Necessary. This change will happen on 18th September 2017.
> >
> > What this means:
> > * No more metadata flags added by Gerrit. There will only be a Change-Id,
> >   Signed-off-by, and BUG (if you've added it). Gerrit itself will not add
> > any
> >   metadata.
> > * If you push a patch on top of another patch, the Submit button will
> > either be
> >   grayed out because the dependent patches cannot be merged or they will be
> >   submited in the correct order in one go.
> >
> > Some of the concerns that have been raised:
> > Q: With the Reviewed-on flag gone, how do we keep track of changesets
> >(especially backports)?
> > A: The Change-Id will get you all the data directly on Gerrit. As long you
> >retain the Change-Id, Gerrit will get you the matching changesets.
> >
> > Q: Will who-wrote-what continue to work?
> > A: As far as I can see, it continues to work. I ran the script against
> >build-jobs repo and it works correctly. Additionally, we'll be setting
> > up an
> >instance of Gerrit Stats[2] to provide more detailed stats.
> >
> > Q: Can we have some of the metadata if not all?
> > Q: Why can't we have the metadata if we change the submit type?
> > A: There's no good answer to this other than, this is how Gerrit works and
> >I can neither change it nor control it.
> >
> >
> One of the major other concerns, which is valid is, stats on 'Reviewed-By'.
> Ie, people who actually reviewed the code. This metrics is equally
> important as 'who-wrote-the-patch', again, getting an infra like [2] below
> will solve it.

The /extras/who-wrote-glusterfs/who-wrote-glusterfs.sh script
will continue to work, but some of the statistics will not be available.
It is nice to have a Reviewed-by tag in the git commit message, but the
one that Gerrit adds is not complete in any case. Reviewers that did not
review the last version of the patch before it got merged do not get
added in the Gerrit-automated-commit-message-ammending. The statistics
of the who-wrote-glusterfs.sh script are cool, and people appreciate
their names in the git logs, but the script should not be used for
serious statistic gathering.

The gerritstats webapp seems much more appropriate for this.

Cheers,
Niels


> 
> Regards,
> Amar
> 
> 
> > [1]: https://review.gluster.org/Documentation/intro-project-
> > owner.html#submit-type
> > [2]: http://gerritstats-demo.firebaseapp.com/
> >
> > --
> > nigelb
> > ___
> > maintainers mailing list
> > maintain...@gluster.org
> > http://lists.gluster.org/mailman/listinfo/maintainers
> >
> 
> 
> 
> -- 
> Amar Tumballi (amarts)

> ___
> Gluster-devel mailing list
> Gluster-devel@gluster.org
> http://lists.gluster.org/mailman/listinfo/gluster-devel

___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel

Re: [Gluster-devel] Changing Submit Type on review.gluster.org

[Niels de Vos]

On Thu, Sep 07, 2017 at 11:50:21AM +0530, Nigel Babu wrote:
> Hello folks,
> 
> A few times, we've merged dependent patches out of order because the Submit
> type[1] did not block us from doing so. The last few times we've talked about
> this, we didn't actually take a strong decision either way. In yesterday's
> maintainers meeting, we agreed to change the Submit type to
> Rebase-If-Necessary. This change will happen on 18th September 2017.
> 
> What this means:
> * No more metadata flags added by Gerrit. There will only be a Change-Id,
>   Signed-off-by, and BUG (if you've added it). Gerrit itself will not add any
>   metadata.
> * If you push a patch on top of another patch, the Submit button will either 
> be
>   grayed out because the dependent patches cannot be merged or they will be
>   submited in the correct order in one go.
> 
> Some of the concerns that have been raised:
> Q: With the Reviewed-on flag gone, how do we keep track of changesets
>(especially backports)?
> A: The Change-Id will get you all the data directly on Gerrit. As long you
>retain the Change-Id, Gerrit will get you the matching changesets.
> 
> Q: Will who-wrote-what continue to work?
> A: As far as I can see, it continues to work. I ran the script against
>build-jobs repo and it works correctly. Additionally, we'll be setting up 
> an
>instance of Gerrit Stats[2] to provide more detailed stats.
> 
> Q: Can we have some of the metadata if not all?
> Q: Why can't we have the metadata if we change the submit type?
> A: There's no good answer to this other than, this is how Gerrit works and
>I can neither change it nor control it.

Q: Can patches of a series be merged before all patches in the series
have a +2? Initial changes that prepare things, or add new (unused) core
functionalities should be mergable so that follow-up patches can be
posted against the HEAD of the branch.

A: Nigel?

Thanks,
Niels


> 
> [1]: 
> https://review.gluster.org/Documentation/intro-project-owner.html#submit-type
> [2]: http://gerritstats-demo.firebaseapp.com/
> 
> --
> nigelb
> ___
> Gluster-devel mailing list
> Gluster-devel@gluster.org
> http://lists.gluster.org/mailman/listinfo/gluster-devel
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel

Re: [Gluster-devel] [Gluster-Maintainers] Changing Submit Type on review.gluster.org

[Milind Changire]

*Squashed Patches*
I believe, individual engineers have to own the responsibility of
maintaining history of all appropriate Change-Ids as part of the commit
message when multiple patches have been squashed/merged into one commit.




On Thu, Sep 7, 2017 at 11:50 AM, Nigel Babu  wrote:

> Hello folks,
>
> A few times, we've merged dependent patches out of order because the Submit
> type[1] did not block us from doing so. The last few times we've talked
> about
> this, we didn't actually take a strong decision either way. In yesterday's
> maintainers meeting, we agreed to change the Submit type to
> Rebase-If-Necessary. This change will happen on 18th September 2017.
>
> What this means:
> * No more metadata flags added by Gerrit. There will only be a Change-Id,
>   Signed-off-by, and BUG (if you've added it). Gerrit itself will not add
> any
>   metadata.
> * If you push a patch on top of another patch, the Submit button will
> either be
>   grayed out because the dependent patches cannot be merged or they will be
>   submited in the correct order in one go.
>
> Some of the concerns that have been raised:
> Q: With the Reviewed-on flag gone, how do we keep track of changesets
>(especially backports)?
> A: The Change-Id will get you all the data directly on Gerrit. As long you
>retain the Change-Id, Gerrit will get you the matching changesets.
>
> Q: Will who-wrote-what continue to work?
> A: As far as I can see, it continues to work. I ran the script against
>build-jobs repo and it works correctly. Additionally, we'll be setting
> up an
>instance of Gerrit Stats[2] to provide more detailed stats.
>
> Q: Can we have some of the metadata if not all?
> Q: Why can't we have the metadata if we change the submit type?
> A: There's no good answer to this other than, this is how Gerrit works and
>I can neither change it nor control it.
>
> [1]: https://review.gluster.org/Documentation/intro-project-
> owner.html#submit-type
> [2]: http://gerritstats-demo.firebaseapp.com/
>
> --
> nigelb
> ___
> maintainers mailing list
> maintain...@gluster.org
> http://lists.gluster.org/mailman/listinfo/maintainers
>



-- 
Milind
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel

Re: [Gluster-devel] [Gluster-Maintainers] Changing Submit Type on review.gluster.org

[Nigel Babu]

On Thu, Sep 07, 2017 at 12:17:32PM +0530, Atin Mukherjee wrote:
> One basic question (rather clarification) here. If indeed a rebase is
> necessary for a patch which was posted some time back and a regression was
> passed at that time, with this change will a (centos) regression job
> re-triggered and once we have a positive vote then only the patch will be
> in to the repo?

There's two cases here:
* The patch does not have any merge conflicts. In this case, Gerrit will do the
  rebase when you hit the submit button. There's no question of running
  regressions. Note: If the patch doesn't have any merge conflicts, you can
  still do a rebase using Gerrit UI or git command line. This will cause the
  regression votes to be retained, but the smoke jobs will be retriggered.
* The patch has merge conflicts. In this case, you'll have to rebase on the git
  command line locally and push up a fix. In this case, regressions votes are
  not carried over and will be triggered.

--
nigelb
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel

Re: [Gluster-devel] Build on FreeBSD and gcc version

[Niels de Vos]

On Wed, Sep 06, 2017 at 12:39:42PM +0200, Michael Scherer wrote:
> Hi,
> 
> so I have been trying to make the internal freebsd builder usable, sinc
> eit was freshly installed and not building anything.
> 
> Over the course of the day, I found a few missing deps, found a ton of
> warnings (I started to fix the easy one, not sure what to do for the
> more complicated one around gf_log formating), but yesterday, I
> stumbled on a more puzzling issue:
> 
>  CCLD glusterfsd
> ../../contrib/argp-standalone/libargp.a(argp-help.o): In function
> `_help':
> /tmp/glusterfs/contrib/argp-standalone/argp-help.c:1608: undefined
> reference to `argp_fmtstream_set_wmargin'
> /tmp/glusterfs/contrib/argp-standalone/argp-help.c:1622: undefined
> reference to `argp_fmtstream_set_lmargin'

FreeBSD seems to provide argp-standalone as a port. It would be better
to use that instead of our bundled version.
  https://www.freebsd.org/ports/devel.html#argp-standalone-1.3_2


> I also see message like this:
> libtool: warning: relinking 'libgfapi.la'
> *** Warning: Linking the shared library libgfapi.la against the
> *** static library ../../contrib/argp-standalone/libargp.a is not
> portable!

libgfapi should not need to link with libargp. This probably is an error
in LDFLAGS or something.
 
> After looking at the old builder, I found a few differences:
> - old one is FreeBSD 10.1, new one is 10.3
> - old one is using gcc 4.8, new one was on clang.
> 
> So I did install gcc, turn out that this was the version 5 by default
> on FreeBSD and it still do not work with it.
> 
> Then I forced gcc4.8 and now it build fine.
> Even better, once I forced the version, it also build (but I suspect
> that's just some bad cleanup on my side, more tests are needed)
> 
> Any reason on why this would fail on gcc 5, and not 4.8, and what we
> should do for that ?

I don't know of any reason, but we should fix it somehow.

> I also found that what fail is to build only in Jenkins, most likely
> due to the setup where we build with a out of source tree directory for
> binary. It work fine if doing ./configure ; make ; make install  

Yeah, this is one of the things that people want to see supported
(although I don't remember the reason). It breaks every now and then :-/

HTH,
Niels
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel

Re: [Gluster-devel] [Gluster-Maintainers] Changing Submit Type on review.gluster.org

[Atin Mukherjee]

On Thu, Sep 7, 2017 at 11:50 AM, Nigel Babu  wrote:

> Hello folks,
>
> A few times, we've merged dependent patches out of order because the Submit
> type[1] did not block us from doing so. The last few times we've talked
> about
> this, we didn't actually take a strong decision either way. In yesterday's
> maintainers meeting, we agreed to change the Submit type to
> Rebase-If-Necessary. This change will happen on 18th September 2017.
>

One basic question (rather clarification) here. If indeed a rebase is
necessary for a patch which was posted some time back and a regression was
passed at that time, with this change will a (centos) regression job
re-triggered and once we have a positive vote then only the patch will be
in to the repo?


> What this means:
> * No more metadata flags added by Gerrit. There will only be a Change-Id,
>   Signed-off-by, and BUG (if you've added it). Gerrit itself will not add
> any
>   metadata.
> * If you push a patch on top of another patch, the Submit button will
> either be
>   grayed out because the dependent patches cannot be merged or they will be
>   submited in the correct order in one go.
>
> Some of the concerns that have been raised:
> Q: With the Reviewed-on flag gone, how do we keep track of changesets
>(especially backports)?
> A: The Change-Id will get you all the data directly on Gerrit. As long you
>retain the Change-Id, Gerrit will get you the matching changesets.
>
> Q: Will who-wrote-what continue to work?
> A: As far as I can see, it continues to work. I ran the script against
>build-jobs repo and it works correctly. Additionally, we'll be setting
> up an
>instance of Gerrit Stats[2] to provide more detailed stats.
>
> Q: Can we have some of the metadata if not all?
> Q: Why can't we have the metadata if we change the submit type?
> A: There's no good answer to this other than, this is how Gerrit works and
>I can neither change it nor control it.
>
> [1]: https://review.gluster.org/Documentation/intro-project-
> owner.html#submit-type
> [2]: http://gerritstats-demo.firebaseapp.com/
>
> --
> nigelb
> ___
> maintainers mailing list
> maintain...@gluster.org
> http://lists.gluster.org/mailman/listinfo/maintainers
>
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel

Re: [Gluster-devel] [Gluster-Maintainers] Changing Submit Type on review.gluster.org

[Amar Tumballi]

On Thu, Sep 7, 2017 at 11:50 AM, Nigel Babu  wrote:

> Hello folks,
>
> A few times, we've merged dependent patches out of order because the Submit
> type[1] did not block us from doing so. The last few times we've talked
> about
> this, we didn't actually take a strong decision either way. In yesterday's
> maintainers meeting, we agreed to change the Submit type to
> Rebase-If-Necessary. This change will happen on 18th September 2017.
>
> What this means:
> * No more metadata flags added by Gerrit. There will only be a Change-Id,
>   Signed-off-by, and BUG (if you've added it). Gerrit itself will not add
> any
>   metadata.
> * If you push a patch on top of another patch, the Submit button will
> either be
>   grayed out because the dependent patches cannot be merged or they will be
>   submited in the correct order in one go.
>
> Some of the concerns that have been raised:
> Q: With the Reviewed-on flag gone, how do we keep track of changesets
>(especially backports)?
> A: The Change-Id will get you all the data directly on Gerrit. As long you
>retain the Change-Id, Gerrit will get you the matching changesets.
>
> Q: Will who-wrote-what continue to work?
> A: As far as I can see, it continues to work. I ran the script against
>build-jobs repo and it works correctly. Additionally, we'll be setting
> up an
>instance of Gerrit Stats[2] to provide more detailed stats.
>
> Q: Can we have some of the metadata if not all?
> Q: Why can't we have the metadata if we change the submit type?
> A: There's no good answer to this other than, this is how Gerrit works and
>I can neither change it nor control it.
>
>
One of the major other concerns, which is valid is, stats on 'Reviewed-By'.
Ie, people who actually reviewed the code. This metrics is equally
important as 'who-wrote-the-patch', again, getting an infra like [2] below
will solve it.

Regards,
Amar


> [1]: https://review.gluster.org/Documentation/intro-project-
> owner.html#submit-type
> [2]: http://gerritstats-demo.firebaseapp.com/
>
> --
> nigelb
> ___
> maintainers mailing list
> maintain...@gluster.org
> http://lists.gluster.org/mailman/listinfo/maintainers
>



-- 
Amar Tumballi (amarts)
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel

[Gluster-devel] Changing Submit Type on review.gluster.org

[Nigel Babu]

Hello folks,

A few times, we've merged dependent patches out of order because the Submit
type[1] did not block us from doing so. The last few times we've talked about
this, we didn't actually take a strong decision either way. In yesterday's
maintainers meeting, we agreed to change the Submit type to
Rebase-If-Necessary. This change will happen on 18th September 2017.

What this means:
* No more metadata flags added by Gerrit. There will only be a Change-Id,
  Signed-off-by, and BUG (if you've added it). Gerrit itself will not add any
  metadata.
* If you push a patch on top of another patch, the Submit button will either be
  grayed out because the dependent patches cannot be merged or they will be
  submited in the correct order in one go.

Some of the concerns that have been raised:
Q: With the Reviewed-on flag gone, how do we keep track of changesets
   (especially backports)?
A: The Change-Id will get you all the data directly on Gerrit. As long you
   retain the Change-Id, Gerrit will get you the matching changesets.

Q: Will who-wrote-what continue to work?
A: As far as I can see, it continues to work. I ran the script against
   build-jobs repo and it works correctly. Additionally, we'll be setting up an
   instance of Gerrit Stats[2] to provide more detailed stats.

Q: Can we have some of the metadata if not all?
Q: Why can't we have the metadata if we change the submit type?
A: There's no good answer to this other than, this is how Gerrit works and
   I can neither change it nor control it.

[1]: 
https://review.gluster.org/Documentation/intro-project-owner.html#submit-type
[2]: http://gerritstats-demo.firebaseapp.com/

--
nigelb
___
Gluster-devel mailing list
Gluster-devel@gluster.org
http://lists.gluster.org/mailman/listinfo/gluster-devel