[SRU] Ubuntu16.04 : autofs is extremely long with large number of direct maps

[Hua Zhang]

[Impact]

autofs service in xenial takes extremely long time since it parses the
mount table /etc/mtab in contained_in_local_fs() to determine if the mount
patch would be created on a local file system, that means startup process will read /etc/mtab for every mount, so the performance is extremely low. The customer is complaining about this point, we need to backport the
following upstream patch.

https://git.kernel.org/pub/scm/linux/storage/autofs/autofs.git/commit/?id=67e7d613a4b09eeffc57ab44a7acb52027d897b2

This fix patch removes contained_in_local_fs, and use fs.f_type in is_remote_fstype() to determine if the mount patch would be created on a
 local file system instead.

[Test Case]

 * Create an autofs direct mount map with large direct mappings (eg: 8k), below is the test script I used.

echo "/- auto.direct --timeout 60" >> /etc/auto.master
END=8000
for i in $(seq 1 $END); do echo $i; echo "/test/samba/test"$i" -fstype=cifs,rw,username=hua,password=password ://192.168.99.169/share" >> /etc/auto.direct; done

 * Start autofs service to see if it will take too much time, below is my
 result before/after applying the fix patch.

root@node1:~# time service autofs start
real 3m5.481s
user 0m0.256s
sys 0m0.080s
root@node1:~# time service autofs start
real 0m0.833s
user 0m0.000s
sys 0m0.004s

[Regression Potential]

* We are well aware of the principle of this fix patch - avoiding parsing
the mount table to improve startup time, so seems infinitely better for
all cases.

* This fix from upstream has been backported into Redhat as well, and both
 me and customer have positive test results with automount start timings.

* What releases to fix

$ git tag --contains 67e7d613a4b09eeffc57ab44a7acb52027d897b2
release_5_1_2

$ rmadison autofs5
 autofs5 | 5.1.1-1ubuntu3 | xenial | all
 autofs5 | 5.1.1-1ubuntu3 | yakkety | all
 autofs5 | 5.1.2-1ubuntu1 | zesty | all
 autofs5 | 5.1.2-1ubuntu1 | artful | all

$ rmadison -u debian autofs5
 autofs5 | 5.1.2-1 | unstable | all

- Ubuntu : Only Xenial and Yakkety are affected.
Zesty and Artful already has the upstream fix because they are at version 5.1.2.

- Debian : unstable also has the fix already.

[Other Info]

* Redhat [1] also already backported the following patch, see: https://bugzilla.redhat.com/show_bug.cgi?id=1440769
* This mailing list also discussed this problem, see: http://www.spinics.net/lists/autofs/msg01161.html

[Christian Ehrhardt  (paelzer)]

Hi Hua Zhang,
you asked me to look for a nomination for Xenial.
I quickly looked and while I ack on Xenial nomination I have a few extra requests.

1. the impact isn't clear are we talking seconds, minutes, hours - if you could add a number from your tests that would be great

2. the testcase is rather unclear, imagine an SRU Team member that actually wants to do this - that is a lot of work. I'm sure you have a tst script that sets up those maps and such - please share and adapt here accordingly.

3. regression potential is clearly not "no regression potential" there could be misdetections not getting root ready, there could be issues due to building the maps late. This is not about writing the nicest words to say "no regression" it is about showing you really thought through the regressions you expect might happen in case things go wrong or thre are oversights in backporting.

4. what releases to fix
$ git tag --contains 67e7d613a4b09eeffc57ab44a7acb52027d897b2
release_5_1_2
$ rmadison autofs5
[...]
 autofs5 | 5.1.1-1ubuntu3 | xenial | all
 autofs5 | 5.1.1-1ubuntu3 | yakkety | all
 autofs5 | 5.1.2-1ubuntu1 | zesty | all
 autofs5 | 5.1.2-1ubuntu1 | artful | all
So IMHO Yakkety needs the fix as well, otherwise it will be a regression on upgrade.

5. the debdiff does not mention this bug number to let it be tracked by LP when things move into proposed and updates, please add them

6. the patch in the debdiff had no dep3 header, but IMHO all pacthes should have one http://dep.debian.net/deps/dep3/
At least add bug, Author (you) and Forwarded with a pointer to the upstream patch

I'll try to nominate X&Y but let you do the further work to adapt SRU template. Since I can't upload autofs you eventually need a core-dev anyway, but I hope my quick check help to reduce the round trips you need.

[Nish Aravamudan (nacc)]

Based upon Christian's analysis, 5.1.2 contains the requested fix and is present in 17.04+

[Hua Zhang (zhhuabj)]

Hi Christian,

Sorry for late response because of sprint and pto during this period, I have updated the bug description and debdiff to address your concerns, many thanks.

[Eric Desrochers (slashd)]

[STS Sponsor Feedbacks]

Both debdiff need some small rework...

* Package versions need to be changed :

Currently both debdiff introduce version "5.1.1-1ubuntu4" in Xenial and Yakkety which is not good.
The reason why they are identical for now, it's probably because no changes has been made between when Xenial package has been copied to Yakkety (when it was a devel release still).
Last change before this one was on "Thu, 19 Nov 2015 17:31:09"

rmadison :
 autofs | 5.1.1-1ubuntu3 | xenial | source, amd64, arm64, armhf, i386, powerpc, ppc64el, s390x
 autofs | 5.1.1-1ubuntu3 | yakkety | source, amd64, arm64, armhf, i386, powerpc, ppc64el, s390x

The good versionning in this case is :

Xenial --> "5.1.1-1ubuntu3.1"
Yakkety --> "5.1.1-1ubuntu4"

* Add the LP bug in debian/changelog as follow :

  * Add one upstream patch to improve performance
   - autofs-5.1.1-improve-scalability-of-direct-mount-pat.patch (LP: #1686679)

* For both debdiff(s), the quilt patch doesn't apply cleanly, here's the details :

# quilt push -a
Applying patch autofs-5.1.1-improve-scalability-of-direct-mount-pat.patch
patching file daemon/automount.c
Hunk #1 succeeded at 95 (offset -3 lines).
Hunk #2 succeeded at 126 (offset -3 lines).
patching file include/automount.h
Hunk #1 succeeded at 70 (offset -5 lines).
patching file include/mounts.h
Hunk #1 succeeded at 94 (offset -7 lines).
patching file lib/mounts.c
Hunk #1 succeeded at 943 (offset 2 lines).

* Any particular reason why "autofs-5.1.1-improve-scalability-of-direct-mount-pat.patch" is not set to apply on the top ?

* quilt header must be in DEP3 format

# "quilt header --dep3 -e" will provide you the template but here's one I started for you that you can complete, if not Bug: nor Bug-Debian:, you can remove the lines.

Description : improve scalability of direct mount path component
 With direct mounts, we want to avoid creating path components on remote file systems
 unless that file system is the root file system. The check boils down to allowing the
 mkdir if:
 1/ If it's the root directory, or
 2/ If it's not a remote file system, or
 3/ If it's a remote file system that's the root file system We can do that without
 parsing the mount table and can improve startup time for all cases.
Author: Jeff Mahoney <email address hidden>
Origin: https://git.kernel.org/pub/scm/linux/storage/autofs/autofs.git/commit/?id=67e7d613
Bug: <upstream_bug>, if any
Bug-Ubuntu: https://bugs.launchpad.net/bugs/988397
Bug-Debian: <debian_bug> if any

Let me know when the new debdiff will be ready, and I'll review/sponsor it.

- Eric

[Hua Zhang (zhhuabj)]

Hi Eric,

I have updated debdiff to address your comments, thanks.

[Eric Desrochers (slashd)]

Hi Hua,

Thanks for the quick debdiffs update. It looks good to me now but I still have 2 concerned :

#1 - Why the quilt patch isn't apply on top ? Any particular reason ?
If there is a reason it's totally fine, I just want to raise all the possible SRU team concerned before the final upload in order to have a smooth SRU.

#2 - A "quilt push -a" command revealed a few "Hunk # succeeded at ...."

Seems like "daemon/automount.c" was successfully patched, but it is now patched with an offset of 28 lines (when it applies cleanly when your patch is absent). Seems like the current position of your patch is influencing the application of others.

So basically it returns to concern #1, is this the appropriate position for this patch ? If yes, why (SRU team will most likely ask the same question, so we better have an answer for them) ? If the answer is no, then please make sure your patch is the topmost patch on the current stack of applied patches.

Validation can be made by doing this command :

$ quilt top
<YOUR_PATCH>

* With your patch in current position :
Applying patch remove-kernel-mount.nfs-version-check.patch
patching file daemon/automount.c
Hunk #2 succeeded at 1397 (offset 28 lines).
Hunk #3 succeeded at 2062 (offset 28 lines).
patching file include/mounts.h
patching file modules/replicated.c
patching file lib/mounts.c

* Without your patch :
Applying patch remove-kernel-mount.nfs-version-check.patch
patching file daemon/automount.c
patching file include/mounts.h
patching file modules/replicated.c
patching file lib/mounts.c

[Hua Zhang (zhhuabj)]

Hi Eric,

I have updated debdiff, thank you.

[Eric Desrochers (slashd)]

Both V3 for Xenial and Yakkety has been uploaded.
It is now waiting for an SRU verification member approval for both packages to start to build in -proposed.

Thanks Joshua for reworking the patch.

- Eric

[Łukasz Zemczak (sil2100)]

Hello Hua, or anyone else affected,

Accepted autofs into yakkety-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/autofs/5.1.1-1ubuntu4 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Łukasz Zemczak (sil2100)]

Hello Hua, or anyone else affected,

Accepted autofs into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/autofs/5.1.1-1ubuntu3.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Hua Zhang (zhhuabj)]

Verified successfully on both xenial-proposed and yakkety-proposed.

[Eric Desrochers (slashd)]

Hi Hua,

Can you elaborate with more details, output, ... on how you did the verification of the proposed package.

The SRU verification team will need more details before proceeding to move the package to -updates and this will help me to justify the completion of the SRU with them.

- Eric

[Hua Zhang (zhhuabj)]

Hi Eric,

Below is my test script and data which are the same as the case description, thanks.

# Test script

echo "/- auto.direct --timeout 60" > /etc/auto.master
END=20000
for i in $(seq 1 $END); do echo $i; echo "/test/samba/test"$i" -fstype=cifs,rw,username=hua,password=password ://192.168.99.169/share" >> /etc/auto.direct; done

# Yakkety with 5.1.1-1ubuntu3

root@yakkety:~# apt-cache policy autofs |grep Installed
  Installed: 5.1.1-1ubuntu3
root@yakkety:~# time service autofs restart
real 1m58.780s
user 0m0.012s
sys 0m0.016s
root@yakkety:~# mount |wc -l
20030

# Yakkety with 5.1.1-1ubuntu4

root@yakkety:~# apt-cache policy autofs |grep Installed
  Installed: 5.1.1-1ubuntu4
root@yakkety:~# time service autofs restart
real 0m6.284s
user 0m0.000s
sys 0m0.012s
root@yakkety:~# mount |wc -l
20030

# Xenail with 5.1.1-1ubuntu3

root@node1:~# apt-cache policy autofs |grep Installed
  Installed: 5.1.1-1ubuntu3
root@node1:~# time service autofs restart
Job for autofs.service failed because a timeout was exceeded. See "systemctl status autofs.service" and "journalctl -xe" for details.
real 5m0.397s
user 0m0.208s
sys 0m0.124s
(NOTE: For the out of 'journalctl -xe' pls see: http://paste.ubuntu.com/24953533/
root@node1:~# mount |wc -l
13103

# Xenail with 5.1.1-1ubuntu3.1

root@node1:~# apt-cache policy autofs |grep Installed
  Installed: 5.1.1-1ubuntu3.1
root@node1:~# time service autofs restart
real 0m6.648s
user 0m0.004s
sys 0m0.000s
root@node1:~# mount |wc -l
20038
root@node1:~# time umount -a -t autofs
real 10m7.251s
user 0m19.476s
sys 0m4.436s
root@node1:~# mount |wc -l
38
root@node1:~# time service autofs restart
real 0m12.967s
user 0m0.008s
sys 0m0.000s
root@node1:~# mount |wc -l
20038

[Łukasz Zemczak (sil2100)]

The verification of the Stable Release Update for autofs has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Brian Murray (brian-murray)]

This bug did not have its tasks automatically modified because they are about the autofs5 package, which only exists in Precise, and not the autofs package.