[PATCH 1/4] testbot: Handle staging at the Job level and avoid race conditions.
Francois Gouget
fgouget at codeweavers.com
Wed Jun 6 17:31:43 CDT 2018
When a patch or executable is submitted for testing it is stored in the
staging directory for two reasons, first because the corresponding Job
does not exist yet, and second because the process may not have write
permissions to the Job directory.
So far transfering the file to the Job directory was the responsibility
of the Step that needs it. However this breaks down when more than one
Step needs that file (e.g. a Windows executable build Step and a Wine
testing one).
There is also a (very unlikely) race condition between the scheduler
and the Job creation: the Scheduler could run the first saved Task, mark
it and the Job as completed before the Job's other Steps and Tasks get
saved to the database. Those would then never get run since the Jobs
has completed.
So this patch makes staging the responsibility of the Job and introduces
a staging Status for it.
It also introduces a new Status which indicates the Job is still being
set up to avoid race conditions.
Note that this patch preserves the Step-based staging mechanism for
backward compatibility during the transition.
Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
---
Note:
* This requires updating the database schema and restarting the TestBot
Engine and web server.
* The following patches switch each Job creator to the new scheme.
testbot/bin/CheckForWinetestUpdate.pl | 2 ++
testbot/ddl/update36.sql | 4 +++
testbot/ddl/winetestbot.sql | 2 +-
testbot/lib/WineTestBot/Engine/Scheduler.pm | 30 ++++++++++++++++++++-
testbot/lib/WineTestBot/Jobs.pm | 20 +++++++++++---
testbot/lib/WineTestBot/Patches.pm | 1 +
testbot/web/Submit.pl | 1 +
7 files changed, 54 insertions(+), 6 deletions(-)
create mode 100644 testbot/ddl/update36.sql
diff --git a/testbot/bin/CheckForWinetestUpdate.pl b/testbot/bin/CheckForWinetestUpdate.pl
index f41148f12..17dab8236 100755
--- a/testbot/bin/CheckForWinetestUpdate.pl
+++ b/testbot/bin/CheckForWinetestUpdate.pl
@@ -200,6 +200,7 @@ sub AddJob($$$)
# First create a new job
my $Jobs = CreateJobs();
my $NewJob = $Jobs->Add();
+ $NewJob->Status("queued");
$NewJob->User(GetBatchUser());
$NewJob->Priority($BaseJob && $Bits == 32 ? 8 : 9);
$NewJob->Remarks($Remarks);
@@ -243,6 +244,7 @@ sub AddReconfigJob()
# First create a new job
my $Jobs = CreateJobs();
my $NewJob = $Jobs->Add();
+ $NewJob->Status("queued");
$NewJob->User(GetBatchUser());
$NewJob->Priority(3);
$NewJob->Remarks($Remarks);
diff --git a/testbot/ddl/update36.sql b/testbot/ddl/update36.sql
new file mode 100644
index 000000000..2f4bf7fa2
--- /dev/null
+++ b/testbot/ddl/update36.sql
@@ -0,0 +1,4 @@
+USE winetestbot;
+
+ALTER TABLE Jobs
+ MODIFY Status ENUM('new', 'staging', 'queued', 'running', 'completed', 'badpatch', 'badbuild', 'boterror', 'canceled') NOT NULL;
diff --git a/testbot/ddl/winetestbot.sql b/testbot/ddl/winetestbot.sql
index 9a28385a9..1e7b1edd1 100644
--- a/testbot/ddl/winetestbot.sql
+++ b/testbot/ddl/winetestbot.sql
@@ -113,7 +113,7 @@ CREATE TABLE Jobs
BranchName VARCHAR(20) NOT NULL,
UserName VARCHAR(40) NOT NULL,
Priority INT(1) NOT NULL,
- Status ENUM('queued', 'running', 'completed', 'badpatch', 'badbuild', 'boterror', 'canceled') NOT NULL,
+ Status ENUM('new', 'staging', 'queued', 'running', 'completed', 'badpatch', 'badbuild', 'boterror', 'canceled') NOT NULL,
Remarks VARCHAR(128) NULL,
Submitted DATETIME NOT NULL,
Ended DATETIME NULL,
diff --git a/testbot/lib/WineTestBot/Engine/Scheduler.pm b/testbot/lib/WineTestBot/Engine/Scheduler.pm
index 10c0b70ed..85bc0f397 100644
--- a/testbot/lib/WineTestBot/Engine/Scheduler.pm
+++ b/testbot/lib/WineTestBot/Engine/Scheduler.pm
@@ -30,6 +30,8 @@ WineTestBot::Engine::Scheduler - Schedules the TestBot tasks
use Exporter 'import';
our @EXPORT = qw(ScheduleJobs CheckJobs);
+use File::Copy;
+
use WineTestBot::Config;
use WineTestBot::Engine::Events;
use WineTestBot::Jobs;
@@ -524,11 +526,37 @@ sub _ScheduleTasks($)
# Process the jobs in decreasing priority order
my $JobRank;
my $Jobs = CreateJobs($Sched->{VMs});
- $Jobs->AddFilter("Status", ["queued", "running"]);
+ $Jobs->AddFilter("Status", ["staging", "queued", "running"]);
foreach my $Job (sort CompareJobPriority @{$Jobs->GetItems()})
{
$JobRank++;
+ if ($Job->Status eq "staging")
+ {
+ # Move the file(s) from the staging directory to the job directory
+ my %Staged;
+ my $JobDir = $Job->CreateDir();
+ foreach my $Step (@{$Job->Steps->Clone()->GetItems()})
+ {
+ # Ignore steps that need a file provided by the previous step
+ next if ($Step->PreviousNo or !defined $Step->FileName);
+ # Skip the step if its file has already been staged
+ next if ($Staged{$Step->FileName});
+
+ my $StagingFile = "job". $Job->Id ."_". $Step->FileName;
+ if (move("$DataDir/staging/$StagingFile", "$JobDir/". $Step->FileName))
+ {
+ $Staged{$Step->FileName} = 1;
+ }
+ else
+ {
+ LogMsg "Could not move the '$StagingFile' staging file: $!";
+ }
+ }
+ $Job->Status("queued");
+ $Job->Save();
+ }
+
# The per-step lists of VMs that should be getting ready to run
# before we prepare the next step
my %StepVMs = ("" => []); # no dependency for the first step
diff --git a/testbot/lib/WineTestBot/Jobs.pm b/testbot/lib/WineTestBot/Jobs.pm
index 13a389586..3fd78aaa2 100644
--- a/testbot/lib/WineTestBot/Jobs.pm
+++ b/testbot/lib/WineTestBot/Jobs.pm
@@ -1,6 +1,6 @@
# -*- Mode: Perl; perl-indent-level: 2; indent-tabs-mode: nil -*-
# Copyright 2009 Ge van Geldorp
-# Copyright 2012-2017 Francois Gouget
+# Copyright 2012-2018 Francois Gouget
#
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
@@ -52,7 +52,19 @@ A Job's lifecycle is as follows:
=over
=item *
-A Job is created with Status set to queued which means it is ready to run.
+A Job is created with Status set to new. During this time the process setting
+up the Job can add new Steps and Tasks to it. It can also put the required
+input files into place in the staging directory with a name of the form
+'job<JobID>_<FileName>' where <JobId> is the Id of the new Job, and FileName is
+the name of a file a Step depends on.
+
+=item *
+Once the Job set up is complete its Status is set to staging. This indicates
+that the TestBot Engine can retrieve the files it depends on from the staging
+directory and move them to the Job's directory.
+
+=item *
+Then the Status is set to queued which indicates the Job is ready to run.
=item *
As soon as one of the Step starts running, the Job's Status field is set to
@@ -93,7 +105,7 @@ sub InitializeNew($$)
my ($self, $Collection) = @_;
$self->Branch(CreateBranches()->GetDefaultBranch());
- $self->Status("queued");
+ $self->Status("new");
$self->Submitted(time());
$self->SUPER::InitializeNew($Collection);
@@ -446,7 +458,7 @@ my @PropertyDescriptors = (
CreateItemrefPropertyDescriptor("Branch", "Branch", !1, 1, \&CreateBranches, ["BranchName"]),
CreateItemrefPropertyDescriptor("User", "Author", !1, 1, \&CreateUsers, ["UserName"]),
CreateBasicPropertyDescriptor("Priority", "Priority", !1, 1, "N", 1),
- CreateEnumPropertyDescriptor("Status", "Status", !1, 1, ['queued', 'running', 'completed', 'badpatch', 'badbuild', 'boterror', 'canceled']),
+ CreateEnumPropertyDescriptor("Status", "Status", !1, 1, ['new', 'staging', 'queued', 'running', 'completed', 'badpatch', 'badbuild', 'boterror', 'canceled']),
CreateBasicPropertyDescriptor("Remarks", "Remarks", !1, !1, "A", 128),
CreateBasicPropertyDescriptor("Submitted", "Submitted", !1, !1, "DT", 19),
CreateBasicPropertyDescriptor("Ended", "Ended", !1, !1, "DT", 19),
diff --git a/testbot/lib/WineTestBot/Patches.pm b/testbot/lib/WineTestBot/Patches.pm
index d5be1b355..4f428133f 100644
--- a/testbot/lib/WineTestBot/Patches.pm
+++ b/testbot/lib/WineTestBot/Patches.pm
@@ -166,6 +166,7 @@ sub Submit($$$)
# Create a new job for this patch
my $NewJob = $Jobs->Add();
+ $NewJob->Status("queued");
$NewJob->User($User);
$NewJob->Priority(6);
my $PropertyDescriptor = $Jobs->GetPropertyDescriptorByName("Remarks");
diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl
index ae69e7ce5..8e22621fd 100644
--- a/testbot/web/Submit.pl
+++ b/testbot/web/Submit.pl
@@ -774,6 +774,7 @@ sub OnSubmit($)
# First create a new job
my $Jobs = CreateJobs();
my $NewJob = $Jobs->Add();
+ $NewJob->Status("queued");
$NewJob->User($self->GetCurrentSession()->User);
$NewJob->Priority(5);
if ($self->GetParam("Remarks"))
--
2.17.0
More information about the wine-devel
mailing list