[PATCH 1/1] report-error.bbclass: Added file syncronization.

[PATCH 1/1] report-error.bbclass: Added file syncronization.

[mariano.lopez]

From: Mariano Lopez <mariano.lopez@linux.intel.com>

errorreport_handler would fail if several errors are
triggered at the same time because of two proccess
writting to the same file. This patch add the required
syncronization to handle concurrent process.

[YP #7899]

Signed-off-by: Mariano Lopez <mariano.lopez@linux.intel.com>
---
 meta/classes/report-error.bbclass | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/meta/classes/report-error.bbclass b/meta/classes/report-error.bbclass
index c5aaaa8..11eee9b 100644
--- a/meta/classes/report-error.bbclass
+++ b/meta/classes/report-error.bbclass
@@ -18,7 +18,6 @@ def errorreport_getdata(e):
 def errorreport_savedata(e, newdata, file):
     import json
     logpath = e.data.getVar('ERR_REPORT_DIR', True)
-    bb.utils.mkdirhier(logpath)
     datafile = os.path.join(logpath, file)
     with open(datafile, "w") as f:
         json.dump(newdata, f, indent=4, sort_keys=True)
@@ -27,7 +26,11 @@ def errorreport_savedata(e, newdata, file):
 python errorreport_handler () {
         import json
 
+        logpath = e.data.getVar('ERR_REPORT_DIR', True)
+        datafile = os.path.join(logpath, "error-report.txt")
+
         if isinstance(e, bb.event.BuildStarted):
+            bb.utils.mkdirhier(logpath)
             data = {}
             machine = e.data.getVar("MACHINE", False)
             data['machine'] = machine
@@ -38,7 +41,9 @@ python errorreport_handler () {
             data['failures'] = []
             data['component'] = e.getPkgs()[0]
             data['branch_commit'] = base_detect_branch(e.data) + ": " + base_detect_revision(e.data)
+            lock = bb.utils.lockfile(datafile + '.lock')
             errorreport_savedata(e, data, "error-report.txt")
+            bb.utils.unlockfile(lock)
 
         elif isinstance(e, bb.build.TaskFailed):
             task = e.task
@@ -56,12 +61,16 @@ python errorreport_handler () {
 
             else:
                 taskdata['log'] = "No Log"
+            lock = bb.utils.lockfile(datafile + '.lock')
             jsondata = json.loads(errorreport_getdata(e))
             jsondata['failures'].append(taskdata)
             errorreport_savedata(e, jsondata, "error-report.txt")
+            bb.utils.unlockfile(lock)
 
         elif isinstance(e, bb.event.BuildCompleted):
+            lock = bb.utils.lockfile(datafile + '.lock')
             jsondata = json.loads(errorreport_getdata(e))
+            bb.utils.unlockfile(lock)
             failures = jsondata['failures']
             if(len(failures) > 0):
                 filename = "error_report_" + e.data.getVar("BUILDNAME", False)+".txt"
-- 
1.8.4.5

Re: [PATCH 1/1] report-error.bbclass: Added file syncronization.

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 567 bytes --]

On 26 June 2015 at 06:56, <mariano.lopez@linux.intel.com> wrote:

> errorreport_handler would fail if several errors are
> triggered at the same time because of two proccess
> writting to the same file. This patch add the required
> syncronization to handle concurrent process.
>

The code in the autobuilder that sends error reports doesn't care for the
filenames, just that they're under error-reports/.  Instead of locking
around a single file - which surely will replace errors with lost reports -
just write each report to a unique filename.

Ross

[-- Attachment #2: Type: text/html, Size: 991 bytes --]

Re: [PATCH 1/1] report-error.bbclass: Added file syncronization.

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 876 bytes --]

On Fri, Jun 26, 2015 at 04:17:01PM +0100, Burton, Ross wrote:
> On 26 June 2015 at 06:56, <mariano.lopez@linux.intel.com> wrote:
> 
> > errorreport_handler would fail if several errors are
> > triggered at the same time because of two proccess
> > writting to the same file. This patch add the required
> > syncronization to handle concurrent process.
> >
> 
> The code in the autobuilder that sends error reports doesn't care for the
> filenames, just that they're under error-reports/.  Instead of locking
> around a single file - which surely will replace errors with lost reports -
> just write each report to a unique filename.

This is about multiple TaskFailed events being processed at the same
time, isn't it? So it's still part of the same report and unrelated to
uploading to web app.

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH 1/1] report-error.bbclass: Added file syncronization.

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 345 bytes --]

On 29 June 2015 at 12:18, Martin Jansa <martin.jansa@gmail.com> wrote:

> This is about multiple TaskFailed events being processed at the same
> time, isn't it? So it's still part of the same report and unrelated to
> uploading to web app.
>

In which case the last task wins, as they're all writing to the same
filename right?

Ross

[-- Attachment #2: Type: text/html, Size: 764 bytes --]

Re: [PATCH 1/1] report-error.bbclass: Added file syncronization.

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 651 bytes --]

On Mon, Jun 29, 2015 at 12:43:04PM +0100, Burton, Ross wrote:
> On 29 June 2015 at 12:18, Martin Jansa <martin.jansa@gmail.com> wrote:
> 
> > This is about multiple TaskFailed events being processed at the same
> > time, isn't it? So it's still part of the same report and unrelated to
> > uploading to web app.
> >
> 
> In which case the last task wins, as they're all writing to the same
> filename right?

No, if the locking is implemented correctly.

Even the last task parses the file before appending additional
task failure and dumping it back to the same filename.

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH 1/1] report-error.bbclass: Added file syncronization.

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 264 bytes --]

On 29 June 2015 at 13:45, Martin Jansa <martin.jansa@gmail.com> wrote:

> Even the last task parses the file before appending additional
> task failure and dumping it back to the same filename.
>

You're right Martin, turns out I can't read code...

Ross

[-- Attachment #2: Type: text/html, Size: 669 bytes --]