[PATCH] Allow for simultaneous do_rootfs tasks with rpm

[PATCH] Allow for simultaneous do_rootfs tasks with rpm

[Stephano Cetola]

Give each rootfs its own RPM channel to use.  This puts the RPM metadata
in a private subdirectory of $WORKDIR, rather than living in DEPLOY_DIR
where other tasks may race with it.

This allows us to reduce the time that the rpm.lock is held to only the
time needed to hardlink the RPMs, allowing the majority of the rootfs
operation to run in parallel.

[ YOCTO #9255 ]

Signed-Off-By: Steven Walter <stevenrwalter@gmail.com>
Signed-off-by: Stephano Cetola <stephano.cetola@linux.intel.com>
---
 meta/classes/rootfs_rpm.bbclass |  5 -----
 meta/lib/oe/package_manager.py  | 17 ++++++++++++++---
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/meta/classes/rootfs_rpm.bbclass b/meta/classes/rootfs_rpm.bbclass
index 38b3c99..37730a7 100644
--- a/meta/classes/rootfs_rpm.bbclass
+++ b/meta/classes/rootfs_rpm.bbclass
@@ -24,11 +24,6 @@ do_populate_sdk[depends] += "${RPMROOTFSDEPENDS}"
 do_rootfs[recrdeptask] += "do_package_write_rpm"
 do_rootfs[vardeps] += "PACKAGE_FEED_URIS"
 
-# RPM doesn't work with multiple rootfs generation at once due to collisions in the use of files 
-# in ${DEPLOY_DIR_RPM}. This can be removed if package_update_index_rpm can be called concurrently
-do_rootfs[lockfiles] += "${DEPLOY_DIR_RPM}/rpm.lock"
-do_populate_sdk[lockfiles] += "${DEPLOY_DIR_RPM}/rpm.lock"
-
 python () {
     if d.getVar('BUILD_IMAGES_FROM_FEEDS', True):
         flags = d.getVarFlag('do_rootfs', 'recrdeptask', True)
diff --git a/meta/lib/oe/package_manager.py b/meta/lib/oe/package_manager.py
index 2004a42..8a08503 100644
--- a/meta/lib/oe/package_manager.py
+++ b/meta/lib/oe/package_manager.py
@@ -9,6 +9,7 @@ import collections
 import bb
 import tempfile
 import oe.utils
+import oe.path
 import string
 from oe.gpg_sign import get_signer
 
@@ -175,7 +176,7 @@ class RpmIndexer(Indexer):
             dbpath = os.path.join(self.d.getVar('WORKDIR', True), 'rpmdb', arch)
             if os.path.exists(dbpath):
                 bb.utils.remove(dbpath, True)
-            arch_dir = os.path.join(self.deploy_dir, arch)
+            arch_dir = os.path.join(self.d.getVar('WORKDIR', True), 'rpms', arch)
             if not os.path.isdir(arch_dir):
                 continue
 
@@ -1010,8 +1011,18 @@ class RpmPM(PackageManager):
         ch_already_added = []
         for canonical_arch in platform_extra:
             arch = canonical_arch.split('-')[0]
-            arch_channel = os.path.join(self.deploy_dir, arch)
-            if os.path.exists(arch_channel) and not arch in ch_already_added:
+            arch_channel = os.path.join(self.d.getVar('WORKDIR', True), 'rpms', arch)
+            oe.path.remove(arch_channel)
+            deploy_arch_dir = os.path.join(self.deploy_dir, arch)
+            if not os.path.exists(deploy_arch_dir):
+                    continue
+
+            lockfilename = self.d.getVar('DEPLOY_DIR_RPM', True) + "/rpm.lock"
+            lf = bb.utils.lockfile(lockfilename, False)
+            oe.path.copyhardlinktree(deploy_arch_dir, arch_channel)
+            bb.utils.unlockfile(lf)
+
+            if not arch in ch_already_added:
                 bb.note('Adding Smart channel %s (%s)' %
                         (arch, channel_priority))
                 self._invoke_smart('channel --add %s type=rpm-md baseurl=%s -y'
-- 
2.9.0

Re: [PATCH] Allow for simultaneous do_rootfs tasks with rpm

[Richard Purdie]

On Thu, 2016-07-14 at 14:20 -0700, Stephano Cetola wrote:
> Give each rootfs its own RPM channel to use.  This puts the RPM
> metadata
> in a private subdirectory of $WORKDIR, rather than living in
> DEPLOY_DIR
> where other tasks may race with it.
> 
> This allows us to reduce the time that the rpm.lock is held to only
> the
> time needed to hardlink the RPMs, allowing the majority of the rootfs
> operation to run in parallel.
> 
> [ YOCTO #9255 ]
> 
> Signed-Off-By: Steven Walter <stevenrwalter@gmail.com>
> Signed-off-by: Stephano Cetola <stephano.cetola@linux.intel.com>
> ---
>  meta/classes/rootfs_rpm.bbclass |  5 -----
>  meta/lib/oe/package_manager.py  | 17 ++++++++++++++---
>  2 files changed, 14 insertions(+), 8 deletions(-)

Sadly, much as I'd love to merge this, testing shows we have some
issues.

Firstly, it means we no longer generate indexes in tmp/deploy/rpm and
this breaks -c testimage since some of the tests connect and look for
packages there, e.g. https://autobuilder.yoctoproject.org/main/builders
/nightly-qa-logrotate/builds/851 but many others would have also
failed.

I've also started wondering what happens if we have some scenario where
we have:

bitbake core-image-sato epiphany
<rootfs for core-image-sato starts>
<rebuild for epiphany or one of its dependencies happens>
<old package from epiphany or one of its dependencies is removed from
tmp/deploy/rpm>
<core-image-sato index references this now removed package>

I'd hope the system should handle this since it should only be
referencing things that are direct task dependencies but with
rpm/smart, who knows :/. We should at least see how this fails (if it
fails).

It could get particularly ugly if we use globbing for the rootfs
construction (e.g. *-locale) and the list of packages changed depending
on what else was built. I'd guess we'd already have this problem but
this patch could magnify any problem.

We are going to have to at least solve the testimage package index
problem before we can merge it.

Cheers,

Richard

Re: [PATCH] Allow for simultaneous do_rootfs tasks with rpm

[Stephano Cetola]

On 07/20, Richard Purdie wrote:
> On Thu, 2016-07-14 at 14:20 -0700, Stephano Cetola wrote:
> > Give each rootfs its own RPM channel to use.  This puts the RPM
> > metadata
> > in a private subdirectory of $WORKDIR, rather than living in
> > DEPLOY_DIR
> > where other tasks may race with it.
> > 
> > This allows us to reduce the time that the rpm.lock is held to only
> > the
> > time needed to hardlink the RPMs, allowing the majority of the rootfs
> > operation to run in parallel.
> > 
> > [ YOCTO #9255 ]
> > 
> > Signed-Off-By: Steven Walter <stevenrwalter@gmail.com>
> > Signed-off-by: Stephano Cetola <stephano.cetola@linux.intel.com>
> > ---
> >  meta/classes/rootfs_rpm.bbclass |  5 -----
> >  meta/lib/oe/package_manager.py  | 17 ++++++++++++++---
> >  2 files changed, 14 insertions(+), 8 deletions(-)
> 
> Sadly, much as I'd love to merge this, testing shows we have some
> issues.
> 
> Firstly, it means we no longer generate indexes in tmp/deploy/rpm and
> this breaks -c testimage since some of the tests connect and look for
> packages there, e.g. https://autobuilder.yoctoproject.org/main/builders
> /nightly-qa-logrotate/builds/851 but many others would have also
> failed.
> 

After digging into this I think that I can change the tests so that they
point at the correct location:

e.g.
tmp/work/qemux86-oe-linux/core-image-sato/1.0-r0/rpms/qemux86/repodata

based on the target triplet, image, PV (I'm guessing here), and arch
variables. I realized when digging into this that using "rpms" in the
path (which was a result of this patch) is wrong. It should be "rpm".

Let me know if this sounds right or if I'm dreaming.

> I've also started wondering what happens if we have some scenario where
> we have:
> 
> bitbake core-image-sato epiphany
> <rootfs for core-image-sato starts>
> <rebuild for epiphany or one of its dependencies happens>
> <old package from epiphany or one of its dependencies is removed from
> tmp/deploy/rpm>
> <core-image-sato index references this now removed package>
> 
> I'd hope the system should handle this since it should only be
> referencing things that are direct task dependencies but with
> rpm/smart, who knows :/. We should at least see how this fails (if it
> fails).
> 

When I build these two this morning I got a bunch of failures on
webkit's configure stage. I'll have to look into those.

> It could get particularly ugly if we use globbing for the rootfs
> construction (e.g. *-locale) and the list of packages changed depending
> on what else was built. I'd guess we'd already have this problem but
> this patch could magnify any problem.
> 
> We are going to have to at least solve the testimage package index
> problem before we can merge it.
> 
> Cheers,
> 
> Richard
> 
> 

Re: [PATCH] Allow for simultaneous do_rootfs tasks with rpm

[Richard Purdie]

On Tue, 2016-07-26 at 20:43 -0700, Stephano Cetola wrote:
> On 07/20, Richard Purdie wrote:
> > On Thu, 2016-07-14 at 14:20 -0700, Stephano Cetola wrote:
> > > Give each rootfs its own RPM channel to use.  This puts the RPM
> > > metadata
> > > in a private subdirectory of $WORKDIR, rather than living in
> > > DEPLOY_DIR
> > > where other tasks may race with it.
> > > 
> > > This allows us to reduce the time that the rpm.lock is held to
> > > only
> > > the
> > > time needed to hardlink the RPMs, allowing the majority of the
> > > rootfs
> > > operation to run in parallel.
> > > 
> > > [ YOCTO #9255 ]
> > > 
> > > Signed-Off-By: Steven Walter <stevenrwalter@gmail.com>
> > > Signed-off-by: Stephano Cetola <stephano.cetola@linux.intel.com>
> > > ---
> > >  meta/classes/rootfs_rpm.bbclass |  5 -----
> > >  meta/lib/oe/package_manager.py  | 17 ++++++++++++++---
> > >  2 files changed, 14 insertions(+), 8 deletions(-)
> > 
> > Sadly, much as I'd love to merge this, testing shows we have some
> > issues.
> > 
> > Firstly, it means we no longer generate indexes in tmp/deploy/rpm
> > and
> > this breaks -c testimage since some of the tests connect and look
> > for
> > packages there, e.g. 
> > https://autobuilder.yoctoproject.org/main/builders
> > /nightly-qa-logrotate/builds/851 but many others would have also
> > failed.
> > 
> 
> After digging into this I think that I can change the tests so that
> they
> point at the correct location:
> 
> e.g.
> tmp/work/qemux86-oe-linux/core-image-sato/1.0
> -r0/rpms/qemux86/repodata
> 
> based on the target triplet, image, PV (I'm guessing here), and arch
> variables. I realized when digging into this that using "rpms" in the
> path (which was a result of this patch) is wrong. It should be "rpm".
> 
> Let me know if this sounds right or if I'm dreaming.

Its not the right fix. The tests can be run against a pre-existing
image which wasn't "built" by the same build directory. tmp/deploy/xxx
is therefore the correct location for the packages/index, we just need
to ensure something has built the indexes there.

Cheers,

Richard

Re: [PATCH] Allow for simultaneous do_rootfs tasks with rpm

[Stephano Cetola]

On 07/27, Richard Purdie wrote:
> On Tue, 2016-07-26 at 20:43 -0700, Stephano Cetola wrote:
> > On 07/20, Richard Purdie wrote:
> > > On Thu, 2016-07-14 at 14:20 -0700, Stephano Cetola wrote:
> > > > Give each rootfs its own RPM channel to use.  This puts the RPM
> > > > metadata
> > > > in a private subdirectory of $WORKDIR, rather than living in
> > > > DEPLOY_DIR
> > > > where other tasks may race with it.
> > > > 
> > > > This allows us to reduce the time that the rpm.lock is held to
> > > > only
> > > > the
> > > > time needed to hardlink the RPMs, allowing the majority of the
> > > > rootfs
> > > > operation to run in parallel.
> > > > 
> > > > [ YOCTO #9255 ]
> > > > 
> > > > Signed-Off-By: Steven Walter <stevenrwalter@gmail.com>
> > > > Signed-off-by: Stephano Cetola <stephano.cetola@linux.intel.com>
> > > > ---
> > > >  meta/classes/rootfs_rpm.bbclass |  5 -----
> > > >  meta/lib/oe/package_manager.py  | 17 ++++++++++++++---
> > > >  2 files changed, 14 insertions(+), 8 deletions(-)
> > > 
> > > Sadly, much as I'd love to merge this, testing shows we have some
> > > issues.
> > > 
> > > Firstly, it means we no longer generate indexes in tmp/deploy/rpm
> > > and
> > > this breaks -c testimage since some of the tests connect and look
> > > for
> > > packages there, e.g. 
> > > https://autobuilder.yoctoproject.org/main/builders
> > > /nightly-qa-logrotate/builds/851 but many others would have also
> > > failed.
> > > 
> > 
> > After digging into this I think that I can change the tests so that
> > they
> > point at the correct location:
> > 
> > e.g.
> > tmp/work/qemux86-oe-linux/core-image-sato/1.0
> > -r0/rpms/qemux86/repodata
> > 
> > based on the target triplet, image, PV (I'm guessing here), and arch
> > variables. I realized when digging into this that using "rpms" in the
> > path (which was a result of this patch) is wrong. It should be "rpm".
> > 
> > Let me know if this sounds right or if I'm dreaming.
> 
> Its not the right fix. The tests can be run against a pre-existing
> image which wasn't "built" by the same build directory. tmp/deploy/xxx
> is therefore the correct location for the packages/index, we just need
> to ensure something has built the indexes there.
> 
> Cheers,
> 
> Richard

I see what you are saying. I'll look into smart and createrepo to see
if there is a "smart" way to update the indexes without requiring a
lock. Is there a place in OE where we can inject code to be run "after
all rootfs"?

-S