[PATCH] testbot: Use tighter build timeouts.

Francois Gouget fgouget at codeweavers.com
Fri Sep 14 09:54:00 CDT 2018


There is no need to allow for a full rebuild if the patch only touches a
few modules. This is particularly significant for Wine builds since they
take longer.
Reducing the timeouts ensures the TestBot will not be blocked longer
than necessary should a task get stuck either due to an issue in the
patch, or to a network issue.
This also centralizes all build timeout calcultations in
GetBuildTimeout().
Finally this slightly changes a couple of timeouts in the scheduler and
activity modules because the $BuildTimeout and $ReconfigTimeout
parameters are going away.

Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
---
 testbot/bin/CheckForWinetestUpdate.pl       | 16 ++++--
 testbot/lib/WineTestBot/Activity.pm         |  2 +-
 testbot/lib/WineTestBot/Config.pm           | 53 ++++++++++--------
 testbot/lib/WineTestBot/Engine/Scheduler.pm |  2 +-
 testbot/lib/WineTestBot/PatchUtils.pm       | 62 +++++++++++++++++----
 testbot/lib/WineTestBot/Patches.pm          | 19 ++++---
 testbot/web/Submit.pl                       | 20 ++++++-
 7 files changed, 122 insertions(+), 52 deletions(-)

diff --git a/testbot/bin/CheckForWinetestUpdate.pl b/testbot/bin/CheckForWinetestUpdate.pl
index a7d77f02f..991cdeb0f 100755
--- a/testbot/bin/CheckForWinetestUpdate.pl
+++ b/testbot/bin/CheckForWinetestUpdate.pl
@@ -50,12 +50,13 @@ use HTTP::Response;
 use HTTP::Status;
 
 use WineTestBot::Config;
+use WineTestBot::Engine::Notify;
 use WineTestBot::Jobs;
-use WineTestBot::Users;
 use WineTestBot::Log;
+use WineTestBot::PatchUtils;
+use WineTestBot::Users;
 use WineTestBot::Utils;
 use WineTestBot::VMs;
-use WineTestBot::Engine::Notify;
 
 
 my %WineTestUrls = (
@@ -245,6 +246,9 @@ sub AddJob($$$)
   return 1;
 }
 
+my @ExeBuilds = qw(exe32 exe64);
+my @WineBuilds = qw(win32 wow32 wow64);
+
 sub AddReconfigJob($)
 {
   my ($VMType) = @_;
@@ -283,9 +287,9 @@ sub AddReconfigJob($)
     Debug("  $VMKey $VMType reconfig\n");
     my $Task = $BuildStep->Tasks->Add();
     $Task->VM($VM);
-    $Task->Timeout($VMType eq "wine" ?
-                   3 * $WineReconfigTimeout : # 3 full Wine builds
-                   $ReconfigTimeout);         # 1 overall timeout
+    my $Builds;
+    map { $Builds->{$_} = 1 } ($VMType eq "wine" ? @WineBuilds : @ExeBuilds);
+    $Task->Timeout(GetBuildTimeout(undef, $Builds));
   }
 
   # Save the build step so the others can reference it.
@@ -299,7 +303,7 @@ sub AddReconfigJob($)
   if ($VMType eq "wine")
   {
     # Add steps to run WineTest on Wine
-    foreach my $Build ("win32", "wow32", "wow64")
+    foreach my $Build (@WineBuilds)
     {
       # Add a step to the job
       my $NewStep = $Steps->Add();
diff --git a/testbot/lib/WineTestBot/Activity.pm b/testbot/lib/WineTestBot/Activity.pm
index a8885a4e1..eba45a039 100644
--- a/testbot/lib/WineTestBot/Activity.pm
+++ b/testbot/lib/WineTestBot/Activity.pm
@@ -438,7 +438,7 @@ sub GetStatistics($;$)
     # Of course this only works for statistics about VM operations and running
     # tasks (so running.time, reverting.time, etc) not for those about idle or
     # off VMs (idle.time, etc.).
-    $ActivitySeconds = $Seconds + 60 + max($SuiteTimeout, $ReconfigTimeout);
+    $ActivitySeconds = $Seconds + 60 + max($SuiteTimeout, 2 * $WineBuildTimeout);
   }
   my ($Activity, $Counters) = GetActivity($VMs, $ActivitySeconds);
   $GlobalStats->{"recordgroups.count"} = $Counters->{recordgroups};
diff --git a/testbot/lib/WineTestBot/Config.pm b/testbot/lib/WineTestBot/Config.pm
index 1ffff262f..a83085d44 100644
--- a/testbot/lib/WineTestBot/Config.pm
+++ b/testbot/lib/WineTestBot/Config.pm
@@ -30,10 +30,11 @@ use vars qw (@ISA @EXPORT @EXPORT_OK $UseSSL $LogDir $DataDir $BinDir
              $MaxRevertsWhileRunningVMs $MaxActiveVMs $MaxRunningVMs
              $MaxVMsWhenIdle $SleepAfterRevert $WaitForToolsInVM
              $VMToolTimeout $MaxVMErrors $MaxTaskTries $AdminEMail $RobotEMail
-             $WinePatchToOverride $WinePatchCc $SuiteTimeout $SingleTimeout
-             $BuildTimeout $ReconfigTimeout $WineReconfigTimeout $TimeoutMargin
-             $TagPrefix
-             $MaxUnitSize $ProjectName $PatchesMailingList $LDAPServer
+             $WinePatchToOverride $WinePatchCc
+             $ExeBuildNativeTimeout $ExeBuildTestTimeout $ExeModuleTimeout
+             $WineBuildTimeout $WineModuleTimeout $TimeoutMargin
+             $SuiteTimeout $SingleTimeout $MaxUnitSize
+             $TagPrefix $ProjectName $PatchesMailingList $LDAPServer
              $LDAPBindDN $LDAPSearchBase $LDAPSearchFilter
              $LDAPRealNameAttribute $LDAPEMailAttribute $AgentPort $Tunnel
              $TunnelDefaults $PrettyHostNames $JobPurgeDays
@@ -46,9 +47,10 @@ require Exporter;
              $MaxRunningVMs $MaxVMsWhenIdle $SleepAfterRevert $WaitForToolsInVM
              $VMToolTimeout $MaxVMErrors $MaxTaskTries $AdminEMail
              $RobotEMail $WinePatchToOverride $WinePatchCc $SuiteTimeout
-             $SingleTimeout $BuildTimeout $ReconfigTimeout $WineReconfigTimeout
-             $TimeoutMargin
-             $TagPrefix $MaxUnitSize $ProjectName $PatchesMailingList
+             $ExeBuildNativeTimeout $ExeBuildTestTimeout $ExeModuleTimeout
+             $WineBuildTimeout $WineModuleTimeout $TimeoutMargin
+             $SuiteTimeout $SingleTimeout $MaxUnitSize
+             $TagPrefix $ProjectName $PatchesMailingList
              $LDAPServer $LDAPBindDN $LDAPSearchBase $LDAPSearchFilter
              $LDAPRealNameAttribute $LDAPEMailAttribute $AgentPort $Tunnel
              $TunnelDefaults $PrettyHostNames $JobPurgeDays
@@ -90,24 +92,29 @@ $MaxVMErrors = 3;
 # How many times to run a test that fails before giving up.
 $MaxTaskTries = 3;
 
-# How long to let a test suite run before forcibly shutting it down
-# (in seconds).
-$SuiteTimeout = 30 * 60;
-# How long to let a regular test run before forcibly shutting it down
-# (in seconds).
-$SingleTimeout = 2 * 60;
-# How long to let a regular build run before forcibly shutting it down
-# (in seconds).
-$BuildTimeout = 5 * 60;
-# How long to let a reconfig task run before forcibly shutting it down
-# (in seconds). Note that this includes building the native Wine build tools,
-# and the 32 and 64 bit test executables.
-$ReconfigTimeout = (1 + 2 * 5) * 60;
-# How long to let a full Wine recompilation run before forcibly shutting it
-# down (in seconds).
-$WineReconfigTimeout = 20 * 60;
+# Exe build timeouts (in seconds)
+# - For a full build
+$ExeBuildNativeTimeout = 60;
+$ExeBuildTestTimeout = 2 * 60;
+# - For a single module
+$ExeModuleTimeout = 30;
+
+# Wine build timeouts (in seconds)
+# - For a full build
+$WineBuildTimeout = 20 * 60;
+# - For a single module
+$WineModuleTimeout = 60;
+
 # How much to add to the task timeout to account for file transfers, etc.
+# (in seconds)
 $TimeoutMargin = 2 * 60;
+
+# Test timeouts (in seconds)
+# - For the whole test suite
+$SuiteTimeout = 30 * 60;
+# - For a single test
+$SingleTimeout = 2 * 60;
+
 # Maximum amount of traces for a test unit.
 $MaxUnitSize = 32 * 1024;
 
diff --git a/testbot/lib/WineTestBot/Engine/Scheduler.pm b/testbot/lib/WineTestBot/Engine/Scheduler.pm
index 4c49f5a1a..ffcc3cdbd 100644
--- a/testbot/lib/WineTestBot/Engine/Scheduler.pm
+++ b/testbot/lib/WineTestBot/Engine/Scheduler.pm
@@ -640,7 +640,7 @@ sub _ScheduleTasks($)
         }
 
         # It's not worth preparing the next step for tasks that take so long
-        $StepVMs{$Step} = undef if ($Task->Timeout > $BuildTimeout);
+        $StepVMs{$Step} = undef if ($Task->Timeout > $VMToolTimeout);
 
         my $VMKey = $VM->GetKey();
         if ($VM->Status eq "idle")
diff --git a/testbot/lib/WineTestBot/PatchUtils.pm b/testbot/lib/WineTestBot/PatchUtils.pm
index d399ced54..7049694aa 100644
--- a/testbot/lib/WineTestBot/PatchUtils.pm
+++ b/testbot/lib/WineTestBot/PatchUtils.pm
@@ -31,7 +31,9 @@ the Wine builds.
 =cut
 
 use Exporter 'import';
-our @EXPORT = qw(GetPatchImpact UpdateWineData);
+our @EXPORT = qw(GetPatchImpact UpdateWineData GetBuildTimeout);
+
+use List::Util qw(min max);
 
 use WineTestBot::Config;
 
@@ -166,11 +168,12 @@ sub _HandleFile($$$)
   if ($FilePath =~ m~^(dlls|programs)/([^/]+)/tests/([^/\s]+)$~)
   {
     my ($Root, $Dir, $File) = ($1, $2, $3);
-    $Impacts->{IsWinePatch} = 1;
+    my $Module = ($Root eq "programs") ? "$Dir.exe" : $Dir;
+    $Impacts->{BuildModules}->{$Module} = 1;
     $Impacts->{TestBuild} = 1;
+    $Impacts->{IsWinePatch} = 1;
 
     my $Tests = $Impacts->{Tests};
-    my $Module = ($Root eq "programs") ? "$Dir.exe" : $Dir;
     if (!$Tests->{$Module})
     {
       $Tests->{$Module} = {
@@ -205,8 +208,9 @@ sub _HandleFile($$$)
   {
     my ($Root, $Dir, $File) = ($1, $2, $3);
     my $Module = ($Root eq "programs") ? "$Dir.exe" : $Dir;
-    $Impacts->{IsWinePatch} = 1;
+    $Impacts->{BuildModules}->{$Module} = 1;
     $Impacts->{ModuleBuild} = 1;
+    $Impacts->{IsWinePatch} = 1;
 
     if ($File eq "Makefile.in" and $Change ne "modify")
     {
@@ -263,6 +267,11 @@ sub GetPatchImpact($;$$)
 
   my $Impacts = {
     NoUnits => $NoUnits,
+    # Number of patched test units.
+    UnitCount => 0,
+    # The modules that need a rebuild, even if only for the tests.
+    BuildModules => {},
+    # Information about 'tests' directories.
     Tests => {},
   };
   _LoadWineFiles();
@@ -277,6 +286,9 @@ sub GetPatchImpact($;$$)
       map { $Impacts->{WineFiles}->{$_} = 1 } keys %{$WineFiles};
       map { $Impacts->{WineFiles}->{$_} = 1 } keys %{$PastImpacts->{NewFiles}};
       map { delete $Impacts->{WineFiles}->{$_} } keys %{$PastImpacts->{DeletedFiles}};
+      # Modules impacted by previous parts of a patchset still need to be
+      # rebuilt.
+      $Impacts->{BuildModules} = { %{$PastImpacts->{BuildModules}} };
     }
     else
     {
@@ -341,8 +353,6 @@ sub GetPatchImpact($;$$)
   }
   close($fh);
 
-  $Impacts->{ModuleCount} = 0;
-  $Impacts->{UnitCount} = 0;
   foreach my $TestInfo (values %{$Impacts->{Tests}})
   {
     # For each module, identify modifications to non-C files and helper dlls
@@ -390,14 +400,44 @@ sub GetPatchImpact($;$$)
     }
 
     $TestInfo->{UnitCount} = scalar(keys %{$TestInfo->{Units}});
-    if ($TestInfo->{UnitCount})
-    {
-      $Impacts->{ModuleCount}++;
-      $Impacts->{UnitCount} += $TestInfo->{UnitCount};
-    }
+    $Impacts->{UnitCount} += $TestInfo->{UnitCount};
   }
 
   return $Impacts;
 }
 
+
+#
+# Compute task timeouts based on the patch data
+#
+
+sub GetBuildTimeout($$)
+{
+  my ($Impacts, $Builds) = @_;
+
+  my ($ExeCount, $WineCount);
+  map {$_ =~ /^exe/ ? $ExeCount++ : $WineCount++ } keys %$Builds;
+
+  # Set $ModuleCount to 0 if a full rebuild is needed
+  my $ModuleCount = (!$Impacts or $Impacts->{WineBuild}) ? 0 :
+                    scalar(keys %{$Impacts->{BuildModules}});
+
+  my ($ExeTimeout, $WineTimeout) = (0, 0);
+  if ($ExeCount)
+  {
+    my $OneBuild = $ModuleCount ? $ModuleCount * $ExeModuleTimeout :
+                                  $ExeBuildTestTimeout;
+    $ExeTimeout = ($ModuleCount ? 0 : $ExeBuildNativeTimeout) +
+                  $ExeCount * min($ExeBuildTestTimeout, $OneBuild);
+  }
+  if ($WineCount)
+  {
+    my $OneBuild = $ModuleCount ? $ModuleCount * $WineModuleTimeout :
+                                  $WineBuildTimeout;
+    $WineTimeout = $WineCount * min($WineBuildTimeout, $OneBuild);
+  }
+
+  return $ExeTimeout + $WineTimeout;
+}
+
 1;
diff --git a/testbot/lib/WineTestBot/Patches.pm b/testbot/lib/WineTestBot/Patches.pm
index f5ef431eb..390b1611c 100644
--- a/testbot/lib/WineTestBot/Patches.pm
+++ b/testbot/lib/WineTestBot/Patches.pm
@@ -185,12 +185,6 @@ sub Submit($$$)
     $BuildStep->Type("build");
     $BuildStep->DebugLevel(0);
 
-    # Add build task
-    my $BuildVM = ${$BuildVMs->GetItems()}[0];
-    my $Task = $BuildStep->Tasks->Add();
-    $Task->VM($BuildVM);
-    $Task->Timeout($BuildTimeout);
-
     # Save the build step so the others can reference it.
     my ($ErrKey, $ErrProperty, $ErrMessage) = $Jobs->Save();
     if (defined($ErrMessage))
@@ -200,6 +194,7 @@ sub Submit($$$)
     }
 
     # Create steps for the Windows tests
+    my $Builds;
     foreach my $Module (sort keys %{$Impacts->{Tests}})
     {
       my $TestInfo = $Impacts->{Tests}->{$Module};
@@ -219,6 +214,7 @@ sub Submit($$$)
             $FileName .= "64" if ($Bits eq "64");
             $NewStep->FileName("$FileName.exe");
             $NewStep->FileType("exe$Bits");
+            $Builds->{"exe$Bits"} = 1;
 
             # And a task for each VM
             my $Tasks = $NewStep->Tasks;
@@ -235,6 +231,12 @@ sub Submit($$$)
         }
       }
     }
+
+    # Add the build task
+    my $BuildVM = ${$BuildVMs->GetItems()}[0];
+    my $BuildTask = $BuildStep->Tasks->Add();
+    $BuildTask->VM($BuildVM);
+    $BuildTask->Timeout(GetBuildTimeout($Impacts, $Builds));
   }
 
   my $WineVMs = CreateVMs();
@@ -258,8 +260,9 @@ sub Submit($$$)
       my $Task = $Tasks->Add();
       $Task->VM($VM);
       # Only verify that the win32 version compiles
-      $Task->Timeout($WineReconfigTimeout);
-      $Task->CmdLineArg("win32");
+      my $Builds = { "win32" => 1 };
+      $Task->Timeout(GetBuildTimeout($Impacts, $Builds));
+      $Task->CmdLineArg(join(",", keys %$Builds));
     }
   }
 
diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl
index 440bd8e5b..e755b5758 100644
--- a/testbot/web/Submit.pl
+++ b/testbot/web/Submit.pl
@@ -777,6 +777,13 @@ sub OnSubmit($)
 
   # Add steps and tasks for the 32 and 64-bit tests
   my $FileType = $self->GetParam("FileType");
+  my $Impacts;
+  if ($FileType eq "patch")
+  {
+    my $TmpStagingFullPath = $self->GetTmpStagingFullPath($BaseName);
+    $Impacts = GetPatchImpact($TmpStagingFullPath);
+  }
+
   my $BuildStep;
   foreach my $Bits ("32", "64")
   {
@@ -812,7 +819,10 @@ sub OnSubmit($)
           my $BuildVM = ${$VMs->GetItems()}[0];
           my $Task = $BuildStep->Tasks->Add();
           $Task->VM($BuildVM);
-          $Task->Timeout($BuildTimeout);
+
+          my $Builds = { "exe32" => 1 };
+          $Builds->{"exe64"} = 1 if (defined $self->GetParam("Run64");
+          $Task->Timeout(GetBuildTimeout($Impacts, $Builds));
 
           # Save the build step so the others can reference it
           my ($ErrKey, $ErrProperty, $ErrMessage) = $Jobs->Save();
@@ -861,6 +871,7 @@ sub OnSubmit($)
     {
       next if ($Build eq "wow64" and !defined($self->GetParam("Run64")));
 
+      my $Timeout;
       foreach my $VMKey (@$SortedKeys)
       {
         my $VM = $VMs->GetItem($VMKey);
@@ -878,12 +889,17 @@ sub OnSubmit($)
           $WineStep->ReportSuccessfulTests(defined($self->GetParam("ReportSuccessfulTests")));
           $Tasks = $WineStep->Tasks;
         }
+        if (!defined $Timeout)
+        {
+          my $Builds = { $Build => 1 };
+          $Timeout = GetBuildTimeout($Impacts, $Builds);
+        }
 
         # Then add a task for this VM
         my $Task = $Tasks->Add();
         $Task->VM($VM);
         $Task->CmdLineArg($Build);
-        $Task->Timeout($WineReconfigTimeout);
+        $Task->Timeout($Timeout);
       }
     }
   }
-- 
2.18.0



More information about the wine-devel mailing list