Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ignore setting job summary when oldstatus == newstatus #9884

Merged
merged 1 commit into from
Jan 25, 2021
Merged

ignore setting job summary when oldstatus == newstatus #9884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions nomad/state/state_store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4428,7 +4428,6 @@ func (s *StateStore) setJobStatus(index uint64, txn *txn,

// Capture the current status so we can check if there is a change
oldStatus := job.Status
firstPass := index == job.CreateIndex
newStatus := forceStatus

// If forceStatus is not set, compute the jobs status.
Expand All @@ -4440,12 +4439,8 @@ func (s *StateStore) setJobStatus(index uint64, txn *txn,
}
}

// Fast-path if the job has changed.
// Still update the job summary if necessary.
// Fast-path if the job has not changed.
if oldStatus == newStatus {
if err := s.setJobSummary(txn, job, index, oldStatus, newStatus, firstPass); err != nil {
return err
}
return nil
}

Expand All @@ -4463,13 +4458,13 @@ func (s *StateStore) setJobStatus(index uint64, txn *txn,
}

// Update the children summary
if err := s.setJobSummary(txn, updated, index, oldStatus, newStatus, firstPass); err != nil {
if err := s.setJobSummary(txn, updated, index, oldStatus, newStatus); err != nil {
return fmt.Errorf("job summary update failed %w", err)
}
return nil
}

func (s *StateStore) setJobSummary(txn *txn, updated *structs.Job, index uint64, oldStatus, newStatus string, firstPass bool) error {
func (s *StateStore) setJobSummary(txn *txn, updated *structs.Job, index uint64, oldStatus, newStatus string) error {
if updated.ParentID == "" {
return nil
}
Expand All @@ -4493,7 +4488,7 @@ func (s *StateStore) setJobSummary(txn *txn, updated *structs.Job, index uint64,
children := pSummary.Children

// Decrement old status
if !firstPass {
if oldStatus != "" {
switch oldStatus {
case structs.JobStatusPending:
children.Pending--
Expand Down
57 changes: 23 additions & 34 deletions nomad/state/state_store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1659,6 +1659,7 @@ func TestStateStore_UpsertJob_ChildJob(t *testing.T) {
}

child := mock.Job()
child.Status = ""
child.ParentID = parent.ID
if err := state.UpsertJob(structs.MsgTypeTestSetup, 1001, child); err != nil {
t.Fatalf("err: %v", err)
Expand Down Expand Up @@ -1996,6 +1997,7 @@ func TestStateStore_DeleteJob_ChildJob(t *testing.T) {
}

child := mock.Job()
child.Status = ""
child.ParentID = parent.ID

if err := state.UpsertJob(structs.MsgTypeTestSetup, 999, child); err != nil {
Expand Down Expand Up @@ -4022,6 +4024,7 @@ func TestStateStore_UpsertEvals_Eval_ChildJob(t *testing.T) {
}

child := mock.Job()
child.Status = ""
child.ParentID = parent.ID

if err := state.UpsertJob(structs.MsgTypeTestSetup, 999, child); err != nil {
Expand Down Expand Up @@ -4246,6 +4249,7 @@ func TestStateStore_DeleteEval_ChildJob(t *testing.T) {
}

child := mock.Job()
child.Status = ""
child.ParentID = parent.ID

if err := state.UpsertJob(structs.MsgTypeTestSetup, 999, child); err != nil {
Expand Down Expand Up @@ -4498,6 +4502,7 @@ func TestStateStore_UpdateAllocsFromClient(t *testing.T) {
}

child := mock.Job()
child.Status = ""
child.ParentID = parent.ID
if err := state.UpsertJob(structs.MsgTypeTestSetup, 999, child); err != nil {
t.Fatalf("err: %v", err)
Expand Down Expand Up @@ -5017,57 +5022,41 @@ func TestStateStore_UpsertAlloc_ChildJob(t *testing.T) {
state := testStateStore(t)

parent := mock.Job()
if err := state.UpsertJob(structs.MsgTypeTestSetup, 998, parent); err != nil {
t.Fatalf("err: %v", err)
}
require.NoError(t, state.UpsertJob(structs.MsgTypeTestSetup, 998, parent))

child := mock.Job()
child.Status = ""
child.ParentID = parent.ID

if err := state.UpsertJob(structs.MsgTypeTestSetup, 999, child); err != nil {
t.Fatalf("err: %v", err)
}
require.NoError(t, state.UpsertJob(structs.MsgTypeTestSetup, 999, child))

alloc := mock.Alloc()
alloc.JobID = child.ID
alloc.Job = child

// Create watchsets so we can test that delete fires the watch
ws := memdb.NewWatchSet()
if _, err := state.JobSummaryByID(ws, parent.Namespace, parent.ID); err != nil {
t.Fatalf("bad: %v", err)
}
_, err := state.JobSummaryByID(ws, parent.Namespace, parent.ID)
require.NoError(t, err)

err := state.UpsertAllocs(structs.MsgTypeTestSetup, 1000, []*structs.Allocation{alloc})
if err != nil {
t.Fatalf("err: %v", err)
}
err = state.UpsertAllocs(structs.MsgTypeTestSetup, 1000, []*structs.Allocation{alloc})
require.NoError(t, err)

if !watchFired(ws) {
t.Fatalf("bad")
}
require.True(t, watchFired(ws))

ws = memdb.NewWatchSet()
summary, err := state.JobSummaryByID(ws, parent.Namespace, parent.ID)
if err != nil {
t.Fatalf("err: %v", err)
}
if summary == nil {
t.Fatalf("nil summary")
}
if summary.JobID != parent.ID {
t.Fatalf("bad summary id: %v", parent.ID)
}
if summary.Children == nil {
t.Fatalf("nil children summary")
}
if summary.Children.Pending != 0 || summary.Children.Running != 1 || summary.Children.Dead != 0 {
t.Fatalf("bad children summary: %v", summary.Children)
}
require.NoError(t, err)
require.NotNil(t, summary)

if watchFired(ws) {
t.Fatalf("bad")
}
require.Equal(t, parent.ID, summary.JobID)
require.NotNil(t, summary.Children)

require.Equal(t, int64(0), summary.Children.Pending)
require.Equal(t, int64(1), summary.Children.Running)
require.Equal(t, int64(0), summary.Children.Dead)

require.False(t, watchFired(ws))
}

func TestStateStore_UpdateAlloc_Alloc(t *testing.T) {
Expand Down