Track multiple job versions and introduce a stop state for jobs #2566

dadgar · 2017-04-17T00:02:11Z

This PR introduces tracking of multiple versions of a job and introduces a stop state to the job.

This allows workflows such as:

$ nomad run example.nomad
$ nomad stop example
$ nomad status example

Previously there would be no way to check that status before.

schmichael

Looks great!

schmichael · 2017-04-17T22:37:26Z

nomad/state/state_store.go

+		}
+
+		if _, err = txn.DeleteAll("job_versions", "id", job.ID, job.Version); err != nil {
+			return fmt.Errorf("deleing job versions failed: %v", err)


deleing -> deleting

schmichael · 2017-04-17T23:35:17Z

scheduler/util.go

@@ -21,7 +21,7 @@ type allocTuple struct {
 // a job requires. This is used to do the count expansion.
 func materializeTaskGroups(job *structs.Job) map[string]*structs.TaskGroup {
 	out := make(map[string]*structs.TaskGroup)
-	if job == nil {
+	if job == nil || job.Stop {


You do this enough a Job.Stopped method might be appropriate:

func (j *Job) Stopped() bool { return j == nil || j.Stop }

armon

LGTM!

armon · 2017-04-19T02:46:39Z

nomad/state/state_store.go

+			continue
+		}
+
+		if _, err = txn.DeleteAll("job_versions", "id", job.ID, job.Version); err != nil {


Why not just call DeleteAll with the id_prefix and eliminate the loop?

Because we are doing a prefix match to get all the versions for the ID you may actually delete another job that starts with the same prefix.

armon · 2017-04-19T02:47:19Z

nomad/state/state_store.go

+		return fmt.Errorf("failed to insert job into job_versions table: %v", err)
+	}
+
+	if err := txn.Insert("index", &IndexEntry{"job_versions", index}); err != nil {


Do we use IndexEntry anymore? Or are we still waiting to move to having MemDB manage blocking queries?

Yeah we still need this. It is used to return the index to wait on when the blocking query is against a nonexistent object. Consul does the same but with slightly different semantics (getter methods on state store return the index rather than the endpoints looking it up.)

armon · 2017-04-19T02:48:07Z

nomad/state/state_store.go

+	for i, j := range all {
+		if j.Stable {
+			stableIdx = i
+			break


Won't the break cause the lowest versioned stable job to be retained?

Not in this case. It is highest version first.

armon · 2017-04-19T02:49:39Z

nomad/state/state_store.go

+	}
+
+	// Sort with highest versions first
+	sort.Slice(all, func(i, j int) bool {


Since we are walking the index, isn't it already sorted? If it's low-to-high, a simple reverse might be sufficient.

Will update.

armon · 2017-04-19T02:52:10Z

scheduler/system_sched.go

@@ -83,6 +83,12 @@ func (s *SystemScheduler) Process(eval *structs.Evaluation) error {
 		s.queuedAllocs)
 }

+// isStoppedJob returns if the scheduling is for a stopped job and the scheduler
+// should stop all its allocations.
+func (s *SystemScheduler) isStoppedJob() bool {


You could define this on the Job to avoid double defining in both schedulers (e.g. just check for nil receiver pointer)

armon · 2017-04-19T02:53:07Z

nomad/state/state_store.go

@@ -322,6 +324,7 @@ func (s *StateStore) UpsertJob(index uint64, job *structs.Job) error {
 		job.CreateIndex = index
 		job.ModifyIndex = index
 		job.JobModifyIndex = index
+		job.Version = 0


Should we also set stable to false?

Yeah. Probably shouldn't have added the stable field in this PR as I didn't implement it.

armon · 2017-04-19T02:53:30Z

nomad/state/state_store.go

@@ -311,6 +312,7 @@ func (s *StateStore) UpsertJob(index uint64, job *structs.Job) error {
 		job.CreateIndex = existing.(*structs.Job).CreateIndex
 		job.ModifyIndex = index
 		job.JobModifyIndex = index
+		job.Version = existing.(*structs.Job).Version + 1


How do we update a job to mark it stable without causing the version to change?

Going to be separate fsm/state_store method

armon · 2017-04-19T02:54:01Z

nomad/state/state_store.go

+		}
+	}
+
+	if err := txn.Insert("index", &IndexEntry{"job_summary", index}); err != nil {


Do we need this?

armon · 2017-04-19T02:55:13Z

nomad/fsm.go

+		stopped := current.Copy()
+		stopped.Stop = true
+
+		if err := n.state.UpsertJob(index, stopped); err != nil {


This should not increment the version, may need a new mode for UpsertJob

Or should it increment? I guess you want it for this case, but not for the case of marking stable.

Yeah will cover the stable case in a follow up PR.

armon · 2017-04-19T02:58:19Z

nomad/state/state_store.go

+
+// upsertJobVersion inserts a job into its historic version table and limits the
+// number of job versions that are tracked.
+func (s *StateStore) upsertJobVersion(index uint64, job *structs.Job, txn *memdb.Txn) error {


Should we guard this on the type? For example, batch probably should only retain 1 version for performance, since they are one-shot anyways.

I say we loop back on this a bit latter. Not sure it makes sense to actually enforce the one-shot nature. You may want to change the logging config/service config etc while it is running which is in-place. In that case having versioning is nice as well�.

I think the one-shot behavior we need to enforce is not re-running a dead batch job and not allowing destructive updates.

github-actions · 2023-04-02T02:10:48Z

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

dadgar added 10 commits April 15, 2017 17:08

Job History schema

4b24421

Upsert Job Histories

0c94d48

Histories -> Versions

5c618ab

GetJobVersions server endpoint

b9a8137

Agent API + api package

330800f

non-purge deregisters

950171f

GC and some fixes

5e1e7af

Status shows type of job

376e177

Diff code fixes

ea15f4b

API Docs

000f42a

dadgar requested a review from armon April 17, 2017 00:02

schmichael approved these changes Apr 17, 2017

View reviewed changes

Fix some tests

f8828a5

armon approved these changes Apr 19, 2017

View reviewed changes

Respond to review comments

019bb3a

dadgar merged commit e281c97 into master Apr 19, 2017

dadgar deleted the f-job-versions branch April 19, 2017 18:11

donovanmuller mentioned this pull request Aug 1, 2017

Ensure compatibility with Nomad 0.6.x donovanmuller/spring-cloud-dataflow-server-nomad#11

Open

github-actions bot locked as resolved and limited conversation to collaborators Apr 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Track multiple job versions and introduce a stop state for jobs #2566

Track multiple job versions and introduce a stop state for jobs #2566

dadgar commented Apr 17, 2017

schmichael left a comment

schmichael Apr 17, 2017

schmichael Apr 17, 2017

armon left a comment

armon Apr 19, 2017

dadgar Apr 19, 2017

armon Apr 19, 2017

dadgar Apr 19, 2017

armon Apr 19, 2017

dadgar Apr 19, 2017

armon Apr 19, 2017

dadgar Apr 19, 2017

armon Apr 19, 2017

armon Apr 19, 2017

dadgar Apr 19, 2017

armon Apr 19, 2017

dadgar Apr 19, 2017

armon Apr 19, 2017

armon Apr 19, 2017

armon Apr 19, 2017

dadgar Apr 19, 2017

armon Apr 19, 2017

dadgar Apr 19, 2017

github-actions bot commented Apr 2, 2023

Track multiple job versions and introduce a stop state for jobs #2566

Track multiple job versions and introduce a stop state for jobs #2566

Conversation

dadgar commented Apr 17, 2017

schmichael left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

armon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Apr 2, 2023