func: add new picker dependency #20029

Juanadelacuesta · 2024-02-22T21:18:08Z

This PR introduces the new options for reconciling a reconnecting allocation and its replacement:

Best score (Current implementation)
Keep original
Keep replacement
Keep the one that has run the longest time

It is achieved by adding a new dependency to the allocReconciler that calls the corresponding function depending on the task group's disconnect strategy. For more detailed information, refer to the RFC

It resolves 15144

scheduler/reconnecting_picker/reconnecting_picker.go

nomad/structs/structs.go

scheduler/reconnecting_picker/reconnecting_picker.go

scheduler/reconcile_util.go

nomad/structs/structs.go

nomad/structs/structs_test.go

lgfa29

Exciting stuff! The reconnect logic has always been confusing to me from an user perspective 😅

May main concern here is the pickLongestRunning logic. We may need to take a different approach to make this decision.

nomad/structs/group.go

lgfa29 · 2024-03-06T15:00:53Z

nomad/structs/structs.go

+// ShouldBeReplaced tests an alloc for replace in case of disconnection
+func (a *Allocation) ShouldBeReplaced() *bool {


What does a nil return represent in this case?

It's probably worth documenting it in the docstring since it makes the return value a tri-state.

lgfa29 · 2024-03-06T15:01:34Z

nomad/structs/structs.go

+// GetLeaderTask will return the leader task in the allocation
+// if there is one, otherwise it will return an empty task.
+func (a *Allocation) GetLeaderTask() Task {


Suggested change

// GetLeaderTask will return the leader task in the allocation

// if there is one, otherwise it will return an empty task.

func (a *Allocation) GetLeaderTask() Task {

// LeaderTask will return the leader task in the allocation

// if there is one, otherwise it will return an empty task.

func (a *Allocation) LeaderTask() *Task {

Gettters in Go don't usually have a Get prefix, and we probably want to return the task pointer to avoid a copy?

I was curious about the difference because these assumptions can be tricky to reason about, so I wrote a quick benchmark. Returning the pointer seems to be an order of magnitude faster 😄

BenchmarkTaskReturn-10 29234692 41.52 ns/op BenchmarkTaskReturn2-10 295444098 4.086 ns/op

lgfa29 · 2024-03-06T16:54:31Z

nomad/structs/structs.go

+	switch len(tg.Tasks) {
+	case 0:
+		return task
+
+	case 1:
+		task = *tg.Tasks[0]
+
+	default:
+		for _, t := range tg.Tasks {
+			if t.Leader {
+				task = *t
+				break
+			}
+		}
+	}
+
+	return task


Suggested change

switch len(tg.Tasks) {

case 0:

return task

case 1:

task = *tg.Tasks[0]

default:

for _, t := range tg.Tasks {

if t.Leader {

task = *t

break

}

}

}

return task

switch len(tg.Tasks) {

case 0:

return task

case 1:

return *tg.Tasks[0]

}

for _, t := range tg.Tasks {

if t.Leader {

return *t

}

}

return task

Minor nit-pick, but I think this is a little easier to read?

lgfa29 · 2024-03-06T17:02:17Z

nomad/structs/structs.go

+// LatestStartOfTask returns the time of the last start event for the given task
+// using the allocations TaskStates. If the task has not started, the zero time
+// will be returned.
+func (a *Allocation) LatestStartOfTask(taskName string) time.Time {


Suggested change

// LatestStartOfTask returns the time of the last start event for the given task

// using the allocations TaskStates. If the task has not started, the zero time

// will be returned.

func (a *Allocation) LatestStartOfTask(taskName string) time.Time {

// LastTaskStart returns the time of the last start event for the given task

// using the allocations TaskStates. If the task has not started, the zero time

// will be returned.

func (a *Allocation) LastTaskStart(taskName string) time.Time {

Another nit-pick 😅

This name matches the name of LastUnknown, which returns a similar result. It could also be useful to move the method so it's closer to that one.

nomad/nomad/structs/structs.go

Lines 11394 to 11409 in 3193ac2

// LastUnknown returns the timestamp for the last time the allocation

// transitioned into the unknown client status.

func (a *Allocation) LastUnknown() time.Time {

var lastUnknown time.Time

for _, s := range a.AllocStates {

if s.Field == AllocStateFieldClientStatus &&

s.Value == AllocClientStatusUnknown {

if lastUnknown.IsZero() || lastUnknown.Before(s.Time) {

lastUnknown = s.Time

}

}

}

return lastUnknown.UTC()

}

lgfa29 · 2024-03-06T17:17:50Z

scheduler/reconcile.go

+type ReconnectingPicker interface {
+	PickReconnectingAlloc(disconnect *structs.DisconnectStrategy, original *structs.Allocation, replacement *structs.Allocation) *structs.Allocation
+}


Why do we need this interface? reconnectingpicker.ReconnectingPicker seems to be the only implementation.

I could see a case where the drain strategy could be an interface with one implementation per strategy, but ReconnectingPicker seems to be handling all of them already.

Im aiming at removing the responsibility of the picking from the allocReconciler, that is the main reason of te interface :)

Interfaces in Go are a bit strange, and work differently than in other languages, like Java or C#.

Since they're implicitly defined pre-creating them can often lead to a confusing implementation. Specially for people new to the code base, trying to "jump to definition" in an interface is quite frustrating 😬

Sometimes that's unavoidable because we want to have multiple implementations (like a real thing and a test implementation), or if want to restrict what to expose (like only exposing certain methods of a struct).

But in this case I can't really think of a different implementations of ReconnectingPicker. The entire decision is already encapsulated in the reconnectingpicker.ReconnectingPicker implementation (the fact they have the same name kind of point to that as well 😅).

So we don't need this interface to remove responsibility from the reconciler. Using the concrete struct already accomplishes that. And having it creates unnecessary indirection that makes the code harder to follow.

If, later on, we do find a need for an interface, the implicit nature of Go's interface can help. If the reconciler ends up calling X(), Y(), and Z(), where each behaves differently under some conditions, we can easily add them to a new interface without much code change.

Having that interface in place gives us a define frontier between the alloc reconciler and the reconnecting picker: Imagine we refactor the reconciler and we want to unit test the logic. If we remove the interface, we remove the possibility of having a look at the internals of the reconciliation, we will need to write tests that also take into account the picker logic, no way to isolate components. If we leave the interface where it is, it is the perfect cutting point where we can add a test implementation of the picker just for the purpose of verifying the behaviour of the reconciliation.

The logic in PickReconnectingAlloc is already pretty self-contained, it doesn't access external data and doesn't cause any side-effects, so I'm not sure we would need to isolate it in tests. I would expect the need would have showed up in the PR already.

But nevertheless, the nice things about implicit interfaces is that, in case we do find ourselves needing alternative implementation, the only line we would need to change is the type of reconnectingPicker, everything else stays the same.

nomad/scheduler/reconcile.go

Line 110 in 7a2740a

reconnectingPicker ReconnectingPicker

The part of the code that has the biggest impact (and it's not even that big) on the scenario you mentioned is this one:

nomad/scheduler/reconcile.go

Line 227 in 7a2740a

reconnectingPicker: reconnectingpicker.New(logger),

This is what is preventing us from feeding the reconciler with different implementation because the reconciler itself is making a decision on what to instantiate, instead of the dependency being fed externally, so it's violating the isolation you mentioned.

But, for the same reason we don't need this interface, we don't need to change this logic. If the need does arise to provide alternate implementation, it's pretty straightforward to update the code. But until then, this interfaces creates unnecessary indirection, making the code hard to read.

https://100go.co/5-interface-pollution/ has some good discussion about interfaces in Go.

It is meant to isolate the logic of the reconciler, not of the picker. When we get to the point of refactoring the reconciler, we can add a configuration option to change the picker, as done in other parts of the code. I you ask me, there are not enough interfaces in the nomad code, isolating components is almost impossible in some parts :)

If we're refactoring the reconciler then adding this interface if necessary won't be any extra work. But until then, this interface is not bringing any benefits, and it's making the code harder to read.

lgfa29 · 2024-03-06T17:27:39Z

scheduler/reconnecting_picker/reconnecting_picker.go

+	// Check if the replacement is newer.
+	// Always prefer the replacement if true.
+	replacementIsNewer := replacement.Job.Version > original.Job.Version ||
+		replacement.Job.CreateIndex > original.Job.CreateIndex
+	if replacementIsNewer {
+		rp.logger.Debug("replacement has a newer version, keeping replacement")
+		return replacement
+	}


Suggested change

// Check if the replacement is newer.

// Always prefer the replacement if true.

replacementIsNewer := replacement.Job.Version > original.Job.Version ||

replacement.Job.CreateIndex > original.Job.CreateIndex

if replacementIsNewer {

rp.logger.Debug("replacement has a newer version, keeping replacement")

return replacement

}

// Check if the replacement is for a newer job version.

// Always prefer the replacement if true.

replacementIsNewer := replacement.Job.Version > original.Job.Version ||

replacement.Job.CreateIndex > original.Job.CreateIndex

if replacementIsNewer {

rp.logger.Debug("replacement has a newer job version, keeping replacement")

return replacement

}

I think the replacement itself would always be newer right? 😅

So just making this a bit more explicit.

lgfa29 · 2024-03-06T17:29:59Z

scheduler/reconnecting_picker/reconnecting_picker_test.go

+	if result != replacement {
+		t.Fatalf("expected replacement, got %v", result)
+	}


Suggested change

if result != replacement {

t.Fatalf("expected replacement, got %v", result)

}

must.Eq(t, replacement, result)

I think this works.

Again, I blame coPilot! jejej

lgfa29 · 2024-03-06T17:31:10Z

scheduler/reconnecting_picker/reconnecting_picker_test.go

+	replacement := &structs.Allocation{
+		Job: &structs.Job{
+			Version:     2,
+			CreateIndex: 2,
+		},
+	}


It may be useful to matrix test each field being modified independently. Testing for a change in both can hide bugs in one of the two checks.

lgfa29 · 2024-03-06T17:50:59Z

scheduler/reconnecting_picker/reconnecting_picker.go

+func (rp *ReconnectingPicker) pickLongestRunning(original *structs.Allocation, replacement *structs.Allocation) *structs.Allocation {
+	lt := original.GetLeaderTask()
+
+	// Default to the first task in the group if no leader is found.
+	if lt.Name == "" {
+		lt = *original.Job.LookupTaskGroup(original.TaskGroup).Tasks[0]
+	}
+
+	// If the replacement has a later start time, keep the original.
+	if original.LatestStartOfTask(lt.Name).Sub(replacement.LatestStartOfTask(lt.Name)) < 0 {
+		return original
+	}
+
+	return replacement


There are a few subtleties here that worry me a bit.

Task states are ephemeral, I think we only keep like 10 around? So for a long-living task I would expect the start event to have been long gone, specially if it uses features like template where each change will generate a handful of events. So in a more realistic scenario we would end up with original.LatestStartOfTask and replacement.LatestStartOfTask being both zero 😬

This is only checking the leader task, or the first one, so it's not taking into consideration aspects such as task lifecycle. If the first task is a prestart sidecar, for example, I think this would give unexpected results.

I think a better approach would be use one of LastRestart or StartedAt (or probably a combination of both) from the alloc's TaskState?

nomad/nomad/structs/structs.go

Lines 8816 to 8826 in 3193ac2

// LastRestart is the time the task last restarted. It is updated each time the

// task restarts

LastRestart time.Time

// StartedAt is the time the task is started. It is updated each time the

// task starts

StartedAt time.Time

// FinishedAt is the time at which the task transitioned to dead and will

// not be started again.

FinishedAt time.Time

We would also need to compare all tasks (or maybe at lest filter by lifecycle) so that we get a more general sense of what to consider "longest running" allocation.

Ok, let me add a new filter to use a task from the main cycle :)

Checking for main is good, but I think the biggest issue is relying on a specific task event. We can't really do that since we only store a limited number of them.

I think we need to rethink how we define longest running. The TaskState I mention could be helpful.

Reimplemented! Now it uses the StartAt, Restarts and LastRestart fields.

Co-authored-by: Luiz Aoqui <[email protected]>

… the get leader to use tasks lifecycle

tgross

LGTM! Once that discussion with @lgfa29 about the interface is resolved this should be good-to-go.

nomad/structs/structs.go

nomad/structs/structs_test.go

lgfa29 · 2024-03-11T15:47:17Z

nomad/structs/structs.go

@@ -11074,6 +11074,16 @@ func (a *Allocation) NextRescheduleTimeByTime(t time.Time) (time.Time, bool) {
 	return a.nextRescheduleTime(t, reschedulePolicy)
 }

+func (a *Allocation) RescheduleTimeOnDisconnect(now time.Time) (time.Time, bool) {


It would be good to have a godoc comment on this one. And do we have a path to deprecate the previous behaviour?

nomad/structs/structs.go

lgfa29 · 2024-03-11T15:54:18Z

nomad/structs/structs.go

+	var task *Task
+
+	// flag used to avoid traversing the tasks twice in case there is no defined
+	// leader.
+	mainTaskFound := false
+
+	for _, t := range tg.Tasks {
+		if t.Leader {
+			task = t
+			break
+		}
+
+		if !mainTaskFound && t.IsMain() {
+			mainTaskFound = true
+			task = t
+		}
+	}
+
+	return task


Suggested change

var task *Task

// flag used to avoid traversing the tasks twice in case there is no defined

// leader.

mainTaskFound := false

for _, t := range tg.Tasks {

if t.Leader {

task = t

break

}

if !mainTaskFound && t.IsMain() {

mainTaskFound = true

task = t

}

}

return task

for _, t := range tg.Tasks {

if t.Leader || t.IsMain() {

return t

}

}

return nil

I think this does the same thing?

And this function is used in pickLongestRunning, but it doesn't seem guarantee that the task returned is the longest running? If the leader or the first main task restarts the its StartedAt counter would reset.

Would it make sense to instead traverse all main tasks and returns the longest period any of the tasks in the group has been running for?

lgfa29 · 2024-03-11T18:53:14Z

scheduler/reconcile.go

+type ReconnectingPicker interface {
+	PickReconnectingAlloc(disconnect *structs.DisconnectStrategy, original *structs.Allocation, replacement *structs.Allocation) *structs.Allocation
+}


If we're refactoring the reconciler then adding this interface if necessary won't be any extra work. But until then, this interface is not bringing any benefits, and it's making the code harder to read.

scheduler/reconnecting_picker/reconnecting_picker.go

Co-authored-by: Luiz Aoqui <[email protected]>

… the start time

lgfa29 · 2024-03-13T01:34:36Z

nomad/structs/structs.go

+// LeaderAndMainTasksInGroup returns two slices: one with the leader tasks and
+// another with the main tasks that are not leader in the given task group.
+// If the task group is no longer present in the allocation definition
+// or there are no leaders and the main body the task is empty both slices will
+// be nil. If the task group has only one task, both slices will contain that task.
+//
+// This function is optimized to avoid traversing the task group tasks more than
+// once in case there is no leader defined.
+func (a *Allocation) LeaderAndMainTasksInGroup(tg *TaskGroup) ([]*Task, []*Task) {
+	if tg == nil {
+		return nil, nil
+	}
+
+	switch len(tg.Tasks) {
+	case 0:
+		return nil, nil
+
+	case 1:
+		return tg.Tasks, tg.Tasks
+	}
+
+	var leaderTasks, mainTasks []*Task
+
+	for _, t := range tg.Tasks {
+		if t.Leader {
+			leaderTasks = append(leaderTasks, t)
+			continue
+		}
+
+		if t.IsMain() {
+			mainTasks = append(mainTasks, t)
+		}
+	}
+
+	return leaderTasks, mainTasks
+}


Given that this method is only used in the reconciler (a part of our code that is called very often), it may be better to avoid allocating the slices and have the loop in StartOfOldestTask. Then we only need to keep the oldest time stored.

Some untested sample code:

func (a *Allocation) StartOfOldestTask(tg *TaskGroup) time.Time { now := time.Now().UTC() oldestStart := now for _, t := range tg.Tasks { if t.Leader { return a.LastStartOfTask(t.Name) } if t.IsMain() { ls := a.LastStartOfTask(t.Name) if !ls.IsZero() && ls.Before(oldestStart) { oldestStart = ls } } } return oldestStart }

Since a group can only have a one leader, this also has the added benefit of cutting the loop short as soon as a leader is found.

nomad/nomad/structs/structs.go

Lines 6984 to 6986 in 428103b

if leaderTasks > 1 {

mErr = multierror.Append(mErr, fmt.Errorf("Only one task may be marked as leader"))

}

lgfa29 · 2024-03-13T01:35:56Z

nomad/structs/structs.go

+	for _, task := range tasks {
+		ls := a.LastStartOfTask(task.Name)
+		if !ls.IsZero() && ls.Before(oldestStart) {
+			oldestStart = a.LastStartOfTask(task.Name)


Suggested change

oldestStart = a.LastStartOfTask(task.Name)

oldestStart = ls

We don't need to call the function again.

lgfa29 · 2024-03-13T01:38:18Z

nomad/structs/structs.go

+	if oldestStart == now {
+		return a.LastStartOfTask(tasks[0].Name)
+	}


Why do we return the first one in this case? If no task has started yet I would imagine StartOfOldestTask to return 0 instead of a somewhat random value? 🤔

Zero is the oldest date possible, that can interfere with the logic else where, and doesn't make much sense.
I needed a default value and that one seemed as good as any other.

The zero time in Go is often used as a sentinel for "not set" (https://cs.opensource.google/go/go/+/refs/tags/go1.22.1:src/time/time.go;l=364-365), that's why IsZero() exists and is a handy check to have 😄

I think returning the time for the first task is an unexpected default and, looking at the code again, we're effectively already returning zero here because the only way to enter this conditional is if oldestStart is not updated, which requires at least one task to be non-zero.

So the only path that leads to this return is if all tasks have zero time, so returning the time for the first task will always return zero, but it's less explicit about and harder to reason.

lgfa29

Some small last comments

nomad/structs/structs_test.go

scheduler/reconnecting_picker/reconnecting_picker.go

Co-authored-by: Luiz Aoqui <[email protected]>

This commit introduces the new options for reconciling a reconnecting allocation and its replacement: Best score (Current implementation) Keep original Keep replacement Keep the one that has run the longest time It is achieved by adding a new dependency to the allocReconciler that calls the corresponding function depending on the task group's disconnect strategy. For more detailed information, refer to the new stanza for disconnected clientes RFC. It resolves 15144

Juanadelacuesta force-pushed the f-hg-15144-disconnect-2 branch 2 times, most recently from f7e2b4f to d7f264a Compare February 22, 2024 21:22

vercel bot deployed to Preview – nomad-storybook-and-ui February 22, 2024 21:25 View deployment

func: add new picker dependency

87be0e8

Juanadelacuesta force-pushed the f-hg-15144-disconnect-2 branch from d7f264a to 87be0e8 Compare February 22, 2024 21:26

vercel bot deployed to Preview – nomad-storybook-and-ui February 22, 2024 21:28 View deployment

fix: avoid panic on empty disconnect group

366b9e8

vercel bot deployed to Preview – nomad-storybook-and-ui February 23, 2024 14:53 View deployment

func: add implementation and tests for the picker strategies

678e329

vercel bot deployed to Preview – nomad-storybook-and-ui March 5, 2024 21:50 View deployment

style: linter fix

1da63fb

vercel bot deployed to Preview – nomad-storybook-and-ui March 5, 2024 22:07 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui March 6, 2024 11:38 View deployment

Juanadelacuesta marked this pull request as ready for review March 6, 2024 11:46

Juanadelacuesta requested review from lgfa29, tgross and jrasell March 6, 2024 11:46

func: add tests for new allocation methods

f5a7145

Juanadelacuesta force-pushed the f-hg-15144-disconnect-2 branch from 4c27d05 to f5a7145 Compare March 6, 2024 11:52

Juanadelacuesta commented Mar 6, 2024

View reviewed changes

scheduler/reconnecting_picker/reconnecting_picker.go Show resolved Hide resolved

vercel bot deployed to Preview – nomad-storybook-and-ui March 6, 2024 11:55 View deployment

tgross reviewed Mar 6, 2024

View reviewed changes

lgfa29 reviewed Mar 6, 2024

View reviewed changes

Apply suggestions from code review

d94fb84

Co-authored-by: Luiz Aoqui <[email protected]>

vercel bot deployed to Preview – nomad-storybook-and-ui March 8, 2024 13:15 View deployment

func: update the function to get the last start to noy use states and…

8000e15

… the get leader to use tasks lifecycle

vercel bot deployed to Preview – nomad-storybook-and-ui March 8, 2024 18:31 View deployment

Juanadelacuesta requested review from lgfa29 and tgross March 8, 2024 18:33

tgross approved these changes Mar 8, 2024

View reviewed changes

nomad/structs/structs.go Outdated Show resolved Hide resolved

nomad/structs/structs_test.go Outdated Show resolved Hide resolved

lgfa29 reviewed Mar 11, 2024

View reviewed changes

Juanadelacuesta and others added 3 commits March 12, 2024 10:14

Update nomad/structs/structs.go

7793b9f

Co-authored-by: Luiz Aoqui <[email protected]>

Update scheduler/reconnecting_picker/reconnecting_picker.go

2edcfc1

Co-authored-by: Luiz Aoqui <[email protected]>

Update scheduler/reconnecting_picker/reconnecting_picker.go

333b61f

Co-authored-by: Luiz Aoqui <[email protected]>

vercel bot deployed to Preview – nomad-storybook-and-ui March 12, 2024 09:19 View deployment

func: take into account the oldest task in the group before selecting…

4e5ff84

… the start time

vercel bot deployed to Preview – nomad-storybook-and-ui March 12, 2024 14:56 View deployment

lgfa29 reviewed Mar 13, 2024

View reviewed changes

vercel bot deployed to Preview – nomad-storybook-and-ui March 13, 2024 23:12 View deployment

Juanadelacuesta force-pushed the f-hg-15144-disconnect-2 branch from 22a9129 to 6171214 Compare March 14, 2024 13:41

vercel bot deployed to Preview – nomad-storybook-and-ui March 14, 2024 13:44 View deployment

style: refactor longuestrunning

c43ee72

Juanadelacuesta force-pushed the f-hg-15144-disconnect-2 branch from 6171214 to c43ee72 Compare March 14, 2024 13:46

vercel bot deployed to Preview – nomad-storybook-and-ui March 14, 2024 13:48 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui March 14, 2024 21:21 View deployment

func: use interface to simplify tests for reconciler

899b4e0

Juanadelacuesta force-pushed the f-hg-15144-disconnect-2 branch from da378ef to 899b4e0 Compare March 14, 2024 21:23

vercel bot deployed to Preview – nomad-storybook-and-ui March 14, 2024 21:26 View deployment

Juanadelacuesta requested a review from lgfa29 March 14, 2024 21:40

lgfa29 approved these changes Mar 15, 2024

View reviewed changes

nomad/structs/structs_test.go Outdated Show resolved Hide resolved

scheduler/reconnecting_picker/reconnecting_picker.go Outdated Show resolved Hide resolved

Juanadelacuesta and others added 2 commits March 15, 2024 12:30

Update scheduler/reconnecting_picker/reconnecting_picker.go

1e55846

Co-authored-by: Luiz Aoqui <[email protected]>

Update nomad/structs/structs_test.go

f2bc833

Co-authored-by: Luiz Aoqui <[email protected]>

vercel bot deployed to Preview – nomad-storybook-and-ui March 15, 2024 11:36 View deployment

style: add changelog

2ef6f90

vercel bot deployed to Preview – nomad-storybook-and-ui March 15, 2024 11:44 View deployment

fix: missing dependecy in test

b75211f

vercel bot deployed to Preview – nomad-storybook-and-ui March 15, 2024 12:13 View deployment

Juanadelacuesta merged commit ff72248 into main Mar 15, 2024
19 checks passed

Juanadelacuesta deleted the f-hg-15144-disconnect-2 branch March 15, 2024 12:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

func: add new picker dependency #20029

func: add new picker dependency #20029

Juanadelacuesta commented Feb 22, 2024 •

edited

Loading

lgfa29 left a comment

lgfa29 Mar 6, 2024

lgfa29 Mar 6, 2024

lgfa29 Mar 6, 2024

lgfa29 Mar 6, 2024

lgfa29 Mar 6, 2024

Juanadelacuesta Mar 8, 2024

lgfa29 Mar 8, 2024

Juanadelacuesta Mar 11, 2024

lgfa29 Mar 11, 2024

Juanadelacuesta Mar 11, 2024 •

edited

Loading

lgfa29 Mar 11, 2024

lgfa29 Mar 6, 2024

lgfa29 Mar 6, 2024

Juanadelacuesta Mar 8, 2024

lgfa29 Mar 6, 2024

lgfa29 Mar 6, 2024

Juanadelacuesta Mar 8, 2024

lgfa29 Mar 8, 2024

Juanadelacuesta Mar 11, 2024

tgross left a comment

lgfa29 Mar 11, 2024

lgfa29 Mar 11, 2024

lgfa29 Mar 11, 2024

lgfa29 Mar 11, 2024

lgfa29 Mar 13, 2024

lgfa29 Mar 13, 2024

lgfa29 Mar 13, 2024

Juanadelacuesta Mar 13, 2024 •

edited

Loading

lgfa29 Mar 13, 2024

lgfa29 left a comment

		// ShouldBeReplaced tests an alloc for replace in case of disconnection
		func (a Allocation) ShouldBeReplaced() bool {

	// LastUnknown returns the timestamp for the last time the allocation
	// transitioned into the unknown client status.
	func (a *Allocation) LastUnknown() time.Time {
	var lastUnknown time.Time

	for _, s := range a.AllocStates {
	if s.Field == AllocStateFieldClientStatus &&
	s.Value == AllocClientStatusUnknown {
	if lastUnknown.IsZero() \|\| lastUnknown.Before(s.Time) {
	lastUnknown = s.Time
	}
	}
	}

	return lastUnknown.UTC()
	}

	// LastRestart is the time the task last restarted. It is updated each time the
	// task restarts
	LastRestart time.Time

	// StartedAt is the time the task is started. It is updated each time the
	// task starts
	StartedAt time.Time

	// FinishedAt is the time at which the task transitioned to dead and will
	// not be started again.
	FinishedAt time.Time

	if leaderTasks > 1 {
	mErr = multierror.Append(mErr, fmt.Errorf("Only one task may be marked as leader"))
	}

func: add new picker dependency #20029

func: add new picker dependency #20029

Conversation

Juanadelacuesta commented Feb 22, 2024 • edited Loading

lgfa29 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

Juanadelacuesta Mar 11, 2024 • edited Loading

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

tgross 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

Juanadelacuesta Mar 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgfa29 left a comment

Choose a reason for hiding this comment

Juanadelacuesta commented Feb 22, 2024 •

edited

Loading

Juanadelacuesta Mar 11, 2024 •

edited

Loading

Juanadelacuesta Mar 13, 2024 •

edited

Loading