Skip to contentSkip to conversation

report-error.bbclass: Added file syncronization.

PATCH In review [email protected] 3 participants 6 messages Jun 26, 2015 – Jun 29, 2015 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

[email protected]

author
#1 lore raw

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

From: Mariano Lopez <[email protected]>

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 <[email protected]>
---
 meta/classes/report-error.bbclass | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
meta/classes/report-error.bbclass +10 -1
index c5aaaa8..11eee9b 100644
@@ -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"
Signature · 1 line
1.8.4.5
#2 lore raw
Quoted 5 lines · On 26 June 2015 at 06:56, <[email protected]> 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

#3 lore raw
Quoted 8 lines · Ross
Quoted 5 lines · On 26 June 2015 at 06:56, <[email protected]> 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.

Quoted 4 lines

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.

Signature · 1 line
Martin 'JaMa' Jansa     jabber: [email protected]
#4 lore raw
Quoted 4 lines · Martin Jansa

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

#5 lore raw
Quoted 9 lines · Ross
Quoted 4 lines · Martin Jansa

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.

Signature · 1 line
Martin 'JaMa' Jansa     jabber: [email protected]
#6 lore raw
Quoted 3 lines · Martin Jansa

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