Revert "[Pinpoint] Scheduler checks if job cancelled"
This reverts commit 4dd11e9f540e827b7dde18bc5fe595cb26f50b4b.
Reason for revert: Chromeperf says builds are failing
Original change's description:
> [Pinpoint] Scheduler checks if job cancelled
>
> Scheduler performs an additional check to see if user manually
> canceled job. Additional check necessary due to race condition.
>
> Bug: chromium:1236759
> Change-Id: Ib1730cb6a5f1254d99e2bbe52a9a57e25a5f77ee
> Reviewed-on: https://chromium-review.googlesource.com/c/catapult/+/3262036
> Reviewed-by: John Chen <johnchen@chromium.org>
> Reviewed-by: Ryan Heise <heiserya@google.com>
> Commit-Queue: Leina Sun <sunxiaodi@google.com>
Bug: chromium:1236759
Change-Id: I62a26d9e4d20101fd39e12cbde59ad4bfe195252
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/catapult/+/3285443
Reviewed-by: John Chen <johnchen@chromium.org>
Commit-Queue: John Chen <johnchen@chromium.org>
diff --git a/dashboard/dashboard/pinpoint/models/job.py b/dashboard/dashboard/pinpoint/models/job.py
index e8d95f9..c7ab376 100644
--- a/dashboard/dashboard/pinpoint/models/job.py
+++ b/dashboard/dashboard/pinpoint/models/job.py
@@ -705,14 +705,6 @@
self.task = None # In case an exception is thrown.
try:
- if scheduler.IsStopped(self):
- # When a user manually cancels a Pinpoint job, job.Cancel() is
- # executed, but it is possible for job.Run() to execute simultaneously,
- # causing a race condition. This if statement takes an extra step to
- # ensure the job stops running after a user cancels the job.
- self.cancelled = True
- logging.debug('Pinpoint job forced to stop because job meets cancellation conditions')
- raise errors.BuildCancelled('Pinpoint Job cancelled')
if self.use_execution_engine:
# Treat this as if it's a poll, and run the handler here.
context = task_module.Evaluate(
diff --git a/dashboard/dashboard/pinpoint/models/job_test.py b/dashboard/dashboard/pinpoint/models/job_test.py
index 61c75af..9aadd9a 100644
--- a/dashboard/dashboard/pinpoint/models/job_test.py
+++ b/dashboard/dashboard/pinpoint/models/job_test.py
@@ -19,7 +19,6 @@
from dashboard.pinpoint.models import change
from dashboard.pinpoint.models import errors
from dashboard.pinpoint.models import job
-from dashboard.pinpoint.models import scheduler
from dashboard.pinpoint import test
_CHROMIUM_URL = 'https://chromium.googlesource.com/chromium/src'
@@ -126,7 +125,6 @@
def testStarted_RecoverableError_BacksOff(self):
j = job.Job.New((), (), comparison_mode='performance')
j.Start()
- scheduler.Schedule(j)
j.state.Explore = mock.MagicMock(side_effect=errors.RecoverableError(None))
j._Schedule = mock.MagicMock()
j.put = mock.MagicMock()
@@ -147,7 +145,6 @@
def testStarted_RecoverableError_Resets(self):
j = job.Job.New((), (), comparison_mode='performance')
j.Start()
- scheduler.Schedule(j)
j.state.Explore = mock.MagicMock(side_effect=errors.RecoverableError(None))
j._Schedule = mock.MagicMock()
j.put = mock.MagicMock()
@@ -186,7 +183,6 @@
def testNoBug(self):
j = job.Job.New((), ())
j.Start()
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(self.add_bug_comment.called)
@@ -208,7 +204,6 @@
def testCompletedNoComparison(self):
j = job.Job.New((), (), bug_id=123456)
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -221,7 +216,6 @@
def testCompletedNoDifference(self):
j = job.Job.New((), (), bug_id=123456, comparison_mode='performance')
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -245,7 +239,6 @@
differences.return_value = []
first_or_last_change_failed.return_value = True
j = job.Job.New((), (), bug_id=123456, comparison_mode='performance')
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -276,7 +269,6 @@
}
self.get_issue.return_value = {'status': 'Untriaged'}
j = job.Job.New((), (), bug_id=123456, comparison_mode='performance')
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -322,7 +314,6 @@
}
layered_cache.SetExternal('commit_hash_git_hash', 'chromium:111222')
j = job.Job.New((), (), bug_id=123456, comparison_mode='performance')
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -373,7 +364,6 @@
bug_id=123456,
comparison_mode='performance',
project='chromium')
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -413,7 +403,6 @@
}
self.get_issue.return_value = None
j = job.Job.New((), (), bug_id=123456, comparison_mode='performance')
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -449,7 +438,6 @@
name=reserved_infos.DOCUMENTATION_URLS.name,
test=utils.TestKey('master/bot/benchmark'))
diag.put()
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -489,7 +477,6 @@
}
self.get_issue.return_value = {'status': 'Untriaged'}
j = job.Job.New((), (), bug_id=123456, comparison_mode='performance')
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -533,7 +520,6 @@
}
}
j = job.Job.New((), (), bug_id=123456, comparison_mode='performance')
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -573,7 +559,6 @@
}
self.get_issue.return_value = {'status': 'Fixed'}
j = job.Job.New((), (), bug_id=123456, comparison_mode='performance')
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -635,7 +620,6 @@
)
self.get_issue.return_value = {'status': 'Untriaged'}
j = job.Job.New((), (), bug_id=123456, comparison_mode='performance')
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -677,7 +661,6 @@
commit_as_dict.side_effect = FakeCommitAsDict
self.get_issue.return_value = {'status': 'Untriaged'}
j = job.Job.New((), (), bug_id=123456, comparison_mode='performance')
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -752,7 +735,6 @@
self.get_issue.return_value = {'status': 'Untriaged'}
j = job.Job.New((), (), bug_id=123456, comparison_mode='performance')
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -815,7 +797,6 @@
self.get_issue.return_value = {'status': 'Untriaged'}
j = job.Job.New((), (), bug_id=123456, comparison_mode='performance')
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -862,7 +843,6 @@
self.get_issue.return_value = {'status': 'Untriaged'}
j = job.Job.New((), (), bug_id=123456, comparison_mode='performance')
j.put()
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -924,7 +904,6 @@
self.get_issue.return_value = {'status': 'Untriaged'}
j = job.Job.New((), (), bug_id=123456, comparison_mode='performance')
j.put()
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
self.assertFalse(j.failed)
@@ -942,7 +921,6 @@
mock.MagicMock(side_effect=AssertionError('Error string')))
def testFailed(self):
j = job.Job.New((), (), bug_id=123456)
- scheduler.Schedule(j)
with self.assertRaises(AssertionError):
j.Run()
@@ -961,7 +939,6 @@
mock.MagicMock(side_effect=AssertionError('Error string')))
def testFailed_ExceptionDetailsFieldAdded(self):
j = job.Job.New((), (), bug_id=123456)
- scheduler.Schedule(j)
with self.assertRaises(AssertionError):
j.Run()
@@ -984,7 +961,6 @@
j = job.Job.New((), (),
gerrit_server='https://review.com',
gerrit_change_id='123456')
- scheduler.Schedule(j)
j.Run()
self.ExecuteDeferredTasks('default')
post_change_comment.assert_called_once_with('https://review.com', '123456',
diff --git a/dashboard/dashboard/pinpoint/models/scheduler.py b/dashboard/dashboard/pinpoint/models/scheduler.py
index 63540c3..cc058b3 100644
--- a/dashboard/dashboard/pinpoint/models/scheduler.py
+++ b/dashboard/dashboard/pinpoint/models/scheduler.py
@@ -266,21 +266,6 @@
})
return result
-@ndb.transactional
-def IsStopped(job):
- """Checks if a job has stopped or not. Jobs should be stopped if
- their status in the job queue is not Running or Queued."""
-
- # Take a job and determine the FIFO Queue it's associated to.
- configuration = job.arguments.get('configuration', '(none)')
-
- # Iterate through the queue and see if job is either running or queued
- queue = ConfigurationQueue.GetOrCreateQueue(configuration)
- for queued_job in queue.jobs:
- if queued_job.job_id == job.job_id:
- if queued_job.status in {'Running', 'Queued'}:
- return False
- return True
@ndb.transactional
def Cancel(job):
