Description of problem: Fuse getattr() sends root_lookup() in case of nodeid == 1, instead of sending stat() itself. Additional info: Copy of the mail thread: ----- On Fri, Mar 24, 2017 at 8:12 PM, Zhitao Li zhitaoli1201 wrote: Hello, Amar, It still fails in quorum test. The root inode resolution succeeds, and stat is called. It fails because of quorum check in afr translator. From: Amar Tumballi <atumball> Sent: Friday, March 24, 2017 10:24:53 PM To: Zhitao Li Cc: Niels de Vos; Poornima Gurusiddaiah; Gluster Devel; gluster-users; Zhitao Li; Zhitao Li Subject: Re: Could we not always call lookup for root_dir in fuse_getattr to improve performance? Test this patch https://review.gluster.org/#/c/16945/ On Fri, Mar 24, 2017 at 9:42 AM, Zhitao Li <zhitaoli1201> wrote: Dear Amar, Thanks very much for your kind help! I have seen the patch you submit. I disable the check and do regression test locally. It indeed brings a bug in "tests/basic/afr/quorum.t"(for tests/ec/, there exists failure but it is hard to reappear the failure). For quorum test, it fails in line_no 36. The case is that for a volume with replica 2, a brick is killed and quorum-reads is on, quorum-type is fixed. Stat on the root_dir will fail because of quorum check(afr/afr-read-txn.c). However, lookup will succeed without quorum check. This is what I have got. So the solution maybe more complex than removing the nodeid==1 check directly. Thank you! Best regards, Zhitao Li From: Amar Tumballi <atumball> Sent: Friday, March 24, 2017 9:13:35 PM To: Zhitao Li Cc: Niels de Vos; Poornima Gurusiddaiah; Gluster Devel; gluster-users; Zhitao Li; Zhitao Li Subject: Re: Could we not always call lookup for root_dir in fuse_getattr to improve performance? Went through the code base again. Yes, as you said, we can get rid of fuse_getattr() not checking nodeid==1 path. Patch here: https://review.gluster.org/16944 -Amar On Fri, Mar 24, 2017 at 2:57 AM, Zhitao Li <zhitaoli1201> wrote: Hello, everyone! I am now optimizing the performance of "ls". When there are many little files directly in mount point(root dir of glusterfs), I find that fuse_getattr takes near half time of total "ls". Strictly, the nodeid==1 check in fuse_getattr will call look operation instead of stat, and lookup will always miss the md-cache, so it will do real lookup and cost about 3ms a time in my case. I doubt whether the special check of nodeid==1 is necessary. I disable the check, it works normal for ls. However, in the "tests" of gluster, it fails(quorum.t). In that case, lookup in root_dir is essential. Up to now, we know that lookup bring high cost in fuse_getattr and it is essential in some case. Could anyone give some advice to improve fuse_getattr(lookup in root_dir) without bringing bugs? I have been trying to deal with this for many days. Your help is greatly in need.
REVIEW: https://review.gluster.org/16945 (fuse-bridge: fuse_getattr(): send root lookup() only in case of failure.) posted (#4) for review on master by Amar Tumballi (amarts)
REVIEW: https://review.gluster.org/16945 (fuse-bridge: fuse_getattr(): send root lookup() only in case of failure.) posted (#5) for review on master by Amar Tumballi (amarts)
REVIEW: https://review.gluster.org/16945 (fuse-bridge: fuse_getattr(): send root lookup() only in case of failure.) posted (#6) for review on master by Amar Tumballi (amarts)
COMMIT: https://review.gluster.org/16945 committed in master by Jeff Darcy (jeff.us) ------ commit ba892262ae7b34b0ba632c0306fcd5addff7d682 Author: Amar Tumballi <amarts> Date: Fri Mar 24 10:16:34 2017 -0400 fuse-bridge: fuse_getattr(): send root lookup() only in case of failure. this will make sure we don't fail in any current cases, and also will enable the performance better. Change-Id: Ia421e1913e1b00f0730a004bf7c84bf7e2a62636 BUG: 1437780 Signed-off-by: Amar Tumballi <amarts> Reviewed-on: https://review.gluster.org/16945 Reviewed-by: Niels de Vos <ndevos> NetBSD-regression: NetBSD Build System <jenkins.org> CentOS-regression: Gluster Build System <jenkins.org> Smoke: Gluster Build System <jenkins.org>
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.11.0, please open a new bug report. glusterfs-3.11.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution. [1] http://lists.gluster.org/pipermail/announce/2017-May/000073.html [2] https://www.gluster.org/pipermail/gluster-users/
REVIEW: https://review.gluster.org/19867 (fuse: make sure the send lookup on root instead of getattr()) posted (#5) for review on master by Shyamsundar Ranganathan
Reopened as per latest commit: "The previous fix for this bug was to add the check for revalidation in lookup when it was sent on root. But I had removed the part where getattr is coming on root. The removing was not requried to fix the issue then. Added back this part of the code, to make sure we have proper validation of root inode in many places like acl, etc."
COMMIT: https://review.gluster.org/19867 committed in master by "Shyamsundar Ranganathan" <srangana> with a commit message- fuse: make sure the send lookup on root instead of getattr() This change was done in https://review.gluster.org/16945. While the changes added there were required, it was not necessary to remove the getattr part. As fuse's lookup on root(/) comes as getattr only, this change is very much required. The previous fix for this bug was to add the check for revalidation in lookup when it was sent on root. But I had removed the part where getattr is coming on root. The removing was not requried to fix the issue then. Added back this part of the code, to make sure we have proper validation of root inode in many places like acl, etc. updates: bz#1437780 Change-Id: I859c4ee1a3f407465cbf19f8934530848424ff50 Signed-off-by: Amar Tumballi <amarts>
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-5.0, please open a new bug report. glusterfs-5.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution. [1] https://lists.gluster.org/pipermail/announce/2018-October/000115.html [2] https://www.gluster.org/pipermail/gluster-users/
REVIEW: https://review.gluster.org/22012 (Revert \"fuse: make sure the send lookup on root instead of getattr()\") posted (#1) for review on master by Amar Tumballi