[PATCH 1/2] sstate: Improve move into place op and touch file

[PATCH 1/2] sstate: Improve move into place op and touch file

[Richard Purdie]

Depending on the kernel and coreutils cersion, mv operations test for existence
of files and can potentially race. It also leads to the file always changing which
leads to potential problems if using and NFS share and there are other readers.

Using ln instead means we don't overwrite the file if it already exists meaning
other readers aren't disrupted and should work more reliably on NFS which is used
for sstate on the autobuilder.

Since we're not overwriting files, touch the file to show activity as would have
been done it it were reused from sstate.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/classes/sstate.bbclass | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index 356fe7ec180..b4ffffcd98b 100644
--- a/meta/classes/sstate.bbclass
+++ b/meta/classes/sstate.bbclass
@@ -776,6 +776,7 @@ sstate_task_postfunc[dirs] = "${WORKDIR}"
 sstate_create_package () {
 	# Exit early if it already exists
 	if [ -e ${SSTATE_PKG} ]; then
+		touch ${SSTATE_PKG}
 		return
 	fi
 
@@ -803,10 +804,13 @@ sstate_create_package () {
 	chmod 0664 $TFILE
 	# Skip if it was already created by some other process
 	if [ ! -e ${SSTATE_PKG} ]; then
-		mv -f $TFILE ${SSTATE_PKG}
+		# Move into place using ln to attempt an atomic op.
+		# Abort if it already exists
+		ln $TFILE ${SSTATE_PKG} && rm $TFILE
 	else
 		rm $TFILE
 	fi
+	touch ${SSTATE_PKG}
 }
 
 python sstate_sign_package () {
-- 
2.20.1

[PATCH 2/2] scripts/oe-build-perf-report: Avoid buildstats warning

[Richard Purdie]

When a comparision branch is set we see warnings. If two branches are used,
look for buildstats information from both to avoid this kind of warning:

INFO: Parsing buildstats from 'refs/notes/buildstats/perf-centos7.yoctoproject.org/master-next/qemux86'
WARNING: Buildstats not found for perf-centos7.yoctoproject.org/master/qemux86/56808-g08220028e52992dcba667fc97bf3afe8be1949fb/0
WARNING: Buildstats not found for perf-centos7.yoctoproject.org/master/qemux86/56808-g08220028e52992dcba667fc97bf3afe8be1949fb/1
INFO: Buildstats were missing for some test runs, please run 'git fetch origin refs/notes/buildstats/perf-centos7.yoctoproject.org/master-next/qemux86:refs/notes/buildstats/perf-centos7.yoctoproject.org/master-next/qemux86' and try again

(the warning is due to stats being looked for in both branches, choose
the other branch and different warnings would be shown)

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 scripts/oe-build-perf-report | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/scripts/oe-build-perf-report b/scripts/oe-build-perf-report
index 21bde7e1561..e781f4f03ff 100755
--- a/scripts/oe-build-perf-report
+++ b/scripts/oe-build-perf-report
@@ -372,7 +372,7 @@ def print_html_report(data, id_comp, buildstats):
                                chart_opts=chart_opts))
 
 
-def get_buildstats(repo, notes_ref, revs, outdir=None):
+def get_buildstats(repo, notes_ref, notes_ref2, revs, outdir=None):
     """Get the buildstats from git notes"""
     full_ref = 'refs/notes/' + notes_ref
     if not repo.rev_parse(full_ref):
@@ -391,8 +391,13 @@ def get_buildstats(repo, notes_ref, revs, outdir=None):
         for tag in rev.tags:
             log.debug('    %s', tag)
             try:
-                bs_all = json.loads(repo.run_cmd(['notes', '--ref', notes_ref,
-                                                  'show', tag + '^0']))
+                try:
+                    bs_all = json.loads(repo.run_cmd(['notes', '--ref', notes_ref, 'show', tag + '^0']))
+                except GitError:
+                    if notes_ref2:
+                        bs_all = json.loads(repo.run_cmd(['notes', '--ref', notes_ref2, 'show', tag + '^0']))
+                    else:
+                        raise
             except GitError:
                 log.warning("Buildstats not found for %s", tag)
                 bs_all = {}
@@ -589,9 +594,12 @@ def main(argv=None):
     buildstats = None
     if args.dump_buildstats or args.html:
         outdir = 'oe-build-perf-buildstats' if args.dump_buildstats else None
-        notes_ref = 'buildstats/{}/{}/{}'.format(args.hostname, args.branch,
-                                                 args.machine)
-        buildstats = get_buildstats(repo, notes_ref, [rev_l, rev_r], outdir)
+        notes_ref = 'buildstats/{}/{}/{}'.format(args.hostname, args.branch, args.machine)
+        notes_ref2 = None
+        if args.branch2:
+            notes_ref = 'buildstats/{}/{}/{}'.format(args.hostname, args.branch2, args.machine)
+            notes_ref2 = 'buildstats/{}/{}/{}'.format(args.hostname, args.branch, args.machine)
+        buildstats = get_buildstats(repo, notes_ref, notes_ref2, [rev_l, rev_r], outdir)
 
     # Print report
     if not args.html:
-- 
2.20.1

Re: [PATCH 1/2] sstate: Improve move into place op and touch file

[Khem Raj]

On Sun, Jan 12, 2020 at 7:42 AM Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> Depending on the kernel and coreutils cersion, mv operations test for existence

typo version -> version
> of files and can potentially race. It also leads to the file always changing which
> leads to potential problems if using and NFS share and there are other readers.
>
> Using ln instead means we don't overwrite the file if it already exists meaning
> other readers aren't disrupted and should work more reliably on NFS which is used
> for sstate on the autobuilder.
>
> Since we're not overwriting files, touch the file to show activity as would have
> been done it it were reused from sstate.
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  meta/classes/sstate.bbclass | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
> index 356fe7ec180..b4ffffcd98b 100644
> --- a/meta/classes/sstate.bbclass
> +++ b/meta/classes/sstate.bbclass
> @@ -776,6 +776,7 @@ sstate_task_postfunc[dirs] = "${WORKDIR}"
>  sstate_create_package () {
>         # Exit early if it already exists
>         if [ -e ${SSTATE_PKG} ]; then
> +               touch ${SSTATE_PKG}
>                 return
>         fi
>
> @@ -803,10 +804,13 @@ sstate_create_package () {
>         chmod 0664 $TFILE
>         # Skip if it was already created by some other process
>         if [ ! -e ${SSTATE_PKG} ]; then
> -               mv -f $TFILE ${SSTATE_PKG}
> +               # Move into place using ln to attempt an atomic op.
> +               # Abort if it already exists
> +               ln $TFILE ${SSTATE_PKG} && rm $TFILE
>         else
>                 rm $TFILE
>         fi
> +       touch ${SSTATE_PKG}
>  }
>
>  python sstate_sign_package () {
> --
> 2.20.1
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core