Job History Service

[Jeremydupras]

Description

Creates a history index that records when a job acquires a lock to execute. Upon completion the record is updated with a completion time. The History service is built into the lock service and completely within jobScheduler.

Logic path
->Extension plugin tries to acquire lock
if the lock is acquired then there is a new record added to the index. If not the nothing is added.
->status codes within the record show if the job finished execution
->upon completion the Extension plugin releases the lock and the record is updated to show the job completed

Example of the index contents

{
  "took" : 2,
  "timed_out" : false,
  "_shards" : {
    "total" : 1,
    "successful" : 1,
    "skipped" : 0,
    "failed" : 0
  },
  "hits" : {
    "total" : {
      "value" : 2,
      "relation" : "eq"
    },
    "max_score" : 1.0,
    "hits" : [
      {
        "_index" : ".opendistro-job-scheduler-history",
        "_id" : ".scheduler_sample_extension-jobid1-1755118872",
        "_score" : 1.0,
        "_source" : {
          "job_index_name" : ".scheduler_sample_extension",
          "job_id" : "jobid1",
          "start_time" : 1755118872,
          "completion_status" : 0,
          "end_time" : 1755118872
        }
      },
      {
        "_index" : ".opendistro-job-scheduler-history",
        "_id" : ".scheduler_sample_extension-jobid1-1755118932",
        "_score" : 1.0,
        "_source" : {
          "job_index_name" : ".scheduler_sample_extension",
          "job_id" : "jobid1",
          "start_time" : 1755118932,
          "completion_status" : 0,
          "end_time" : 1755118932
        }
      }
    ]
  }
}

Related Issues

Resolves #808

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.99%. Comparing base (fedf867) to head (91c6e69).
⚠️ Report is 2 commits behind head on main.

❌ Your project status has failed because the head coverage (69.99%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #814      +/-   ##
==========================================
+ Coverage   69.80%   69.99%   +0.18%     
==========================================
  Files          38       38              
  Lines        1729     1733       +4     
  Branches      156      156              
==========================================
+ Hits         1207     1213       +6     
+ Misses        431      430       -1     
+ Partials       91       90       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[DarshitChanpura]

Thank you @Jeremydupras . PR looks close. left few comments. Will take a second pass once all are addressed.

[DarshitChanpura]

why are we awaiting in tests, that too with arbitrary values

[Jeremydupras]

Test are based directly off of the Lock service IT tests. Allows the test time for the async operations.

[DarshitChanpura]

we should not await tests. You handle assertions in the listener block so that should cover the case.

[Jeremydupras]

removed the await

[Jeremydupras]

The historyService has asynchronous methods when writing to in Index. The Await ensures that the method finished before evaluating the result.

[cwperks]

Do we need to keep track of seqNo and primaryTerm. I'd prefer to only keep references to the new logic being introduced and not keep track of these metadata fields.

[Jeremydupras]

The current design adds to the history list when the lock is created and updates the entry when the lock in released. If the sequence number and primary term are not tracked the history service should be changed to append only.

[cwperks]

I don't think its necessary to track these, but I suppose it doesn't hurt either.

FYI I see that the usage is here in the LockService.

UpdateRequest updateRequest = new UpdateRequest().index(LOCK_INDEX_NAME)
                .id(updateLock.getLockId())
                .setIfSeqNo(updateLock.getSeqNo())
                .setIfPrimaryTerm(updateLock.getPrimaryTerm())
                .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
                .doc(updateLock.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS))
                .fetchSource(true);

According to the javadoc, this is the purpose:

/**
   * only perform this update request if the document's modification was assigned the given
   * sequence number. Must be used in combination with {@link #setIfPrimaryTerm(long)}
   *
   * If the document last modification was assigned a different sequence number a
   * {@link org.opensearch.index.engine.VersionConflictEngineException} will be thrown.
   */

[cwperks]

Thank you for the pr @Jeremydupras ! This looks like it would be a nice feature to have and makes sense to introduce it behind a feature flag. There's a couple of comments I left that need to be addressed, but the pr mostly looks good.

[DarshitChanpura]

left small comments, looks good otherwise.

[DarshitChanpura]

is the call on this line independent of the one on line 341?

asking because both are executed in async manner.

[Jeremydupras]

Yes, 341 updates the history index with an end time and success status code. 350 releases the lock from the job which updates the lock index to "released = true"

[DarshitChanpura]

Nice work @Jeremydupras ! LGTM 🎊