diff --git a/src/clusterfuzz/_internal/cron/cleanup.py b/src/clusterfuzz/_internal/cron/cleanup.py index 1fa1abd40e..24571eeb66 100644 --- a/src/clusterfuzz/_internal/cron/cleanup.py +++ b/src/clusterfuzz/_internal/cron/cleanup.py @@ -205,22 +205,29 @@ def cleanup_unused_fuzz_targets_and_jobs(): last_run_cutoff = utils.utcnow() - datetime.timedelta( days=FUZZ_TARGET_UNUSED_THRESHOLD) - unused_target_jobs = data_types.FuzzTargetJob.query( - data_types.FuzzTargetJob.last_run < last_run_cutoff) - # The order by last_run DESC filter is from b/418807403 - valid_target_jobs = list( - data_types.FuzzTargetJob.query( - data_types.FuzzTargetJob.last_run >= last_run_cutoff).order( - -data_types.FuzzTargetJob.last_run)) - - to_delete = [t.key for t in unused_target_jobs] + # Scan in key order and filter client-side: querying the frequently rewritten + # last_run index times out and can skip entities (b/406250205). + unused_target_jobs = [] + valid_fuzz_targets = set() + num_valid_target_jobs = 0 + for target_job in ndb_utils.get_all_from_model(data_types.FuzzTargetJob): + if target_job.last_run is None: + # Matches neither inequality filter of the previous implementation, so + # these are neither deleted nor considered valid. + continue + if target_job.last_run >= last_run_cutoff: + valid_fuzz_targets.add(target_job.fuzz_target_name) + num_valid_target_jobs += 1 + else: + unused_target_jobs.append(target_job.key) + + to_delete = list(unused_target_jobs) num_fuzz_target_jobs_to_delete = len(to_delete) logs.info( - f'{len(to_delete)} FuzzTargetJob entities are marked for deletion and' - f'{len(valid_target_jobs)} are considered valid.') + f'{num_fuzz_target_jobs_to_delete} FuzzTargetJob entities are marked for ' + f'deletion and {num_valid_target_jobs} are considered valid.') - valid_fuzz_targets = {t.fuzz_target_name for t in valid_target_jobs} for fuzz_target in ndb_utils.get_all_from_model(data_types.FuzzTarget): if fuzz_target.fully_qualified_name() not in valid_fuzz_targets: to_delete.append(fuzz_target.key) @@ -229,7 +236,7 @@ def cleanup_unused_fuzz_targets_and_jobs(): logs.info( f'Deleted {num_fuzz_target_jobs_to_delete} FuzzTargetJob entities and ' f'{len(to_delete) - num_fuzz_target_jobs_to_delete} FuzzTarget entities. ' - f'{len(valid_target_jobs)} valid FuzzTargetJob entities remain.') + f'{num_valid_target_jobs} valid FuzzTargetJob entities remain.') def get_jobs_and_platforms_for_project(): diff --git a/src/clusterfuzz/_internal/tests/appengine/handlers/cron/cleanup_test.py b/src/clusterfuzz/_internal/tests/appengine/handlers/cron/cleanup_test.py index f30445aadd..cdc9b2eed5 100644 --- a/src/clusterfuzz/_internal/tests/appengine/handlers/cron/cleanup_test.py +++ b/src/clusterfuzz/_internal/tests/appengine/handlers/cron/cleanup_test.py @@ -2328,6 +2328,20 @@ def test_cleanup_unused_fuzz_targets_and_jobs(self): 'libFuzzer_binary4/job2' ], list([t.key.id() for t in data_types.FuzzTargetJob.query()])) + def test_cleanup_fuzz_target_job_without_last_run(self): + """Test that FuzzTargetJobs without last_run are neither deleted nor + considered valid.""" + data_types.FuzzTarget( + engine='libFuzzer', binary='binary1', project='test-project').put() + data_types.FuzzTargetJob( + fuzz_target_name='libFuzzer_binary1', job='job1', last_run=None).put() + + cleanup.cleanup_unused_fuzz_targets_and_jobs() + + self.assertEqual([], [t.key.id() for t in data_types.FuzzTarget.query()]) + self.assertEqual(['libFuzzer_binary1/job1'], + [t.key.id() for t in data_types.FuzzTargetJob.query()]) + @test_utils.with_cloud_emulators('datastore') class CleanupUnusedHeartbeatsTest(unittest.TestCase):