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@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"))