This is an automated email from the ASF dual-hosted git repository.
ccondit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/yunikorn-core.git
The following commit(s) were added to refs/heads/master by this push:
new 756882ca [YUNIKORN-2641] Ensure createTime is consistent between ask
and alloc (#873)
756882ca is described below
commit 756882ca05957b8c1ccdbe445c92ed4ea3685bf3
Author: Craig Condit <[email protected]>
AuthorDate: Tue May 28 10:38:18 2024 -0500
[YUNIKORN-2641] Ensure createTime is consistent between ask and alloc (#873)
Update the createTime in AllocationAsk and Allocation to be always
populated and never modified.
Closes: #873
---
pkg/scheduler/objects/allocation.go | 30 +++++++++---------------------
pkg/scheduler/objects/allocation_ask.go | 15 ++++++++++++++-
pkg/scheduler/objects/allocation_test.go | 12 ++++--------
pkg/scheduler/objects/application.go | 1 -
4 files changed, 27 insertions(+), 31 deletions(-)
diff --git a/pkg/scheduler/objects/allocation.go
b/pkg/scheduler/objects/allocation.go
index fff75116..adde18b2 100644
--- a/pkg/scheduler/objects/allocation.go
+++ b/pkg/scheduler/objects/allocation.go
@@ -52,17 +52,16 @@ type Allocation struct {
ask *AllocationAsk
allocationKey string
applicationID string
- partitionName string
taskGroupName string // task group this allocation belongs to
placeholder bool // is this a placeholder allocation
nodeID string
priority int32
tags map[string]string
allocatedResource *resources.Resource
+ createTime time.Time // the time this allocation was created
// Mutable fields which need protection
placeholderUsed bool
- createTime time.Time // the time this allocation was created
bindTime time.Time // the time this allocation was bound
to a node
placeholderCreateTime time.Time
released bool
@@ -76,17 +75,11 @@ type Allocation struct {
}
func NewAllocation(nodeID string, ask *AllocationAsk) *Allocation {
- var createTime time.Time
- if ask.GetTag(siCommon.CreationTime) == "" {
- createTime = time.Now()
- } else {
- createTime = ask.GetCreateTime()
- }
return &Allocation{
ask: ask,
allocationKey: ask.GetAllocationKey(),
applicationID: ask.GetApplicationID(),
- createTime: createTime,
+ createTime: ask.GetCreateTime(),
bindTime: time.Now(),
nodeID: nodeID,
tags: ask.GetTagsClone(),
@@ -127,11 +120,14 @@ func NewAllocationFromSI(alloc *si.Allocation)
*Allocation {
return nil
}
- creationTime, err :=
strconv.ParseInt(alloc.AllocationTags[siCommon.CreationTime], 10, 64)
+ var createTime time.Time
+ siCreationTime, err :=
strconv.ParseInt(alloc.AllocationTags[siCommon.CreationTime], 10, 64)
if err != nil {
- log.Log(log.SchedAllocation).Warn("CreationTime is not set on
the Allocation object or invalid",
+ log.Log(log.SchedAllocation).Debug("CreationTime is not set on
the Allocation object or invalid",
zap.String("creationTime",
alloc.AllocationTags[siCommon.CreationTime]))
- creationTime = -1
+ createTime = time.Now()
+ } else {
+ createTime = time.Unix(siCreationTime, 0)
}
ask := &AllocationAsk{
@@ -143,7 +139,7 @@ func NewAllocationFromSI(alloc *si.Allocation) *Allocation {
allocated: true,
taskGroupName: alloc.TaskGroupName,
placeholder: alloc.Placeholder,
- createTime: time.Unix(creationTime, 0),
+ createTime: createTime,
allocLog: make(map[string]*AllocationLogEntry),
originator: alloc.Originator,
allowPreemptSelf: alloc.PreemptionPolicy.GetAllowPreemptSelf(),
@@ -208,17 +204,9 @@ func (a *Allocation) GetTaskGroup() string {
// GetCreateTime returns the time this allocation was created
func (a *Allocation) GetCreateTime() time.Time {
- a.RLock()
- defer a.RUnlock()
return a.createTime
}
-func (a *Allocation) SetCreateTime(createTime time.Time) {
- a.Lock()
- defer a.Unlock()
- a.createTime = createTime
-}
-
// GetBindTime returns the time this allocation was created
func (a *Allocation) GetBindTime() time.Time {
a.RLock()
diff --git a/pkg/scheduler/objects/allocation_ask.go
b/pkg/scheduler/objects/allocation_ask.go
index e213a1b6..341a19e9 100644
--- a/pkg/scheduler/objects/allocation_ask.go
+++ b/pkg/scheduler/objects/allocation_ask.go
@@ -20,6 +20,7 @@ package objects
import (
"fmt"
+ "strconv"
"time"
"go.uber.org/zap"
@@ -29,6 +30,7 @@ import (
"github.com/apache/yunikorn-core/pkg/events"
"github.com/apache/yunikorn-core/pkg/locking"
"github.com/apache/yunikorn-core/pkg/log"
+ siCommon "github.com/apache/yunikorn-scheduler-interface/lib/go/common"
"github.com/apache/yunikorn-scheduler-interface/lib/go/si"
)
@@ -84,12 +86,23 @@ func NewAllocationAsk(allocationKey string, applicationID
string, allocatedResou
}
func NewAllocationAskFromSI(ask *si.AllocationAsk) *AllocationAsk {
+
+ var createTime time.Time
+ siCreationTime, err :=
strconv.ParseInt(ask.Tags[siCommon.CreationTime], 10, 64)
+ if err != nil {
+ log.Log(log.SchedAllocation).Debug("CreationTime is not set on
the AllocationAsk object or invalid",
+ zap.String("creationTime",
ask.Tags[siCommon.CreationTime]))
+ createTime = time.Now()
+ } else {
+ createTime = time.Unix(siCreationTime, 0)
+ }
+
saa := &AllocationAsk{
allocationKey: ask.AllocationKey,
allocatedResource:
resources.NewResourceFromProto(ask.ResourceAsk),
applicationID: ask.ApplicationID,
tags: CloneAllocationTags(ask.Tags),
- createTime: time.Now(),
+ createTime: createTime,
priority: ask.Priority,
placeholder: ask.Placeholder,
taskGroupName: ask.TaskGroupName,
diff --git a/pkg/scheduler/objects/allocation_test.go
b/pkg/scheduler/objects/allocation_test.go
index 475ba9d6..a12df946 100644
--- a/pkg/scheduler/objects/allocation_test.go
+++ b/pkg/scheduler/objects/allocation_test.go
@@ -65,13 +65,6 @@ func TestNewAlloc(t *testing.T) {
expected := "applicationID=app-1, allocationKey=ask-1, Node=node-1,
result=Allocated"
assert.Equal(t, allocStr, expected, "Strings should have been equal")
assert.Assert(t, !alloc.IsPlaceholderUsed(), fmt.Sprintf("Alloc should
not be placeholder replacement by default: got %t, expected %t",
alloc.IsPlaceholderUsed(), false))
- created := alloc.GetCreateTime()
- // move time 10 seconds back
- alloc.SetCreateTime(created.Add(time.Second * -10))
- createdNow := alloc.GetCreateTime()
- if createdNow.Equal(created) {
- t.Fatal("create time stamp should have been modified")
- }
// check that createTime is properly copied from the ask
tags := make(map[string]string)
tags[siCommon.CreationTime] = strconv.FormatInt(past, 10)
@@ -211,8 +204,11 @@ func TestNewAllocFromSI(t *testing.T) {
allocSI.PreemptionPolicy.AllowPreemptSelf = true
allocSI.PreemptionPolicy.AllowPreemptOther = false
allocSI.AllocationTags[siCommon.CreationTime] = "xyz"
+ startTime := time.Now().Unix()
alloc = NewAllocationFromSI(allocSI)
- assert.Equal(t, alloc.GetAsk().GetCreateTime().Unix(), int64(-1))
//nolint:staticcheck
+ endTime := time.Now().Unix()
+ assert.Assert(t, alloc.GetAsk().GetCreateTime().Unix() >= startTime,
"alloc create time is too early")
+ assert.Assert(t, alloc.GetAsk().GetCreateTime().Unix() <= endTime,
"alloc create time is too late")
assert.Assert(t, !alloc.GetAsk().IsOriginator(), "ask should not have
been an originator")
assert.Assert(t, alloc.GetAsk().IsAllowPreemptSelf(), "ask should have
allow-preempt-self set")
assert.Assert(t, !alloc.GetAsk().IsAllowPreemptOther(), "ask should not
have allow-preempt-other set")
diff --git a/pkg/scheduler/objects/application.go
b/pkg/scheduler/objects/application.go
index 5ef35dea..b76466be 100644
--- a/pkg/scheduler/objects/application.go
+++ b/pkg/scheduler/objects/application.go
@@ -1741,7 +1741,6 @@ func (sa *Application) ReplaceAllocation(allocationKey
string) *Allocation {
alloc := ph.GetFirstRelease()
alloc.SetPlaceholderUsed(true)
alloc.SetPlaceholderCreateTime(ph.GetCreateTime())
- alloc.SetCreateTime(time.Now())
alloc.SetBindTime(time.Now())
sa.addAllocationInternal(alloc)
// order is important: clean up the allocation after adding it to the
app
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
