If we used PodBackoffMap only for the PriorityQueue, we wouldn't need this here. podInfo already has a timestamp that can be used. For now, we can have it here until we remove the FIFO queue in the next step.

this PR is using podLastUpdateTime + backoffDuration --> pod completes backoff time
where backoffDuration is calculated from numberOfAttempts
shall we change to podInfo.timestamp + backoffDuration --> pod completes backoff time?
i'm think podInfo.timestamp is the time pod added to this scheduling queue, while podLastUpdateTime is the time pod starts backoff. please correct me if i misunderstood sth. thanks.

ok i see, in AddUnschedulableIfNotPresent we have pInfo.timestamp equal to podLastUpdateTime

func (p *PriorityQueue) AddUnschedulableIfNotPresent(pod *v1.Pod, podSchedulingCycle int64) error {
        ......
	pInfo := p.newPodInfo(pod)
        ......
	// Every unschedulable pod is subject to backoff timers.
	p.backoffPod(pod)
        ......
}

maybe we could update the comment below from The time pod added to the scheduling queue. to The time pod last updated in the scheduling queue?

// podInfo is minimum cell in the scheduling queue.
type podInfo struct {
	pod *v1.Pod
	// The time pod added to the scheduling queue.
	timestamp time.Time
}

Yes. podInfo.timestamp can represent the time pod starts backoff in AddUnschedulableIfNotPresent . But the comment is not needed to change. Because timestamp represents the time pod added to the scheduling queue(no matter activeQ、podBackoffQ、unschedulableQ)

I see.
shall we change this PR to use podInfo here?
i am thinking for PodBackoffMap to perform backoff, we only need namespace_pod as the key of the map, instead of all the info from *v1.Pod in podInfo.

shall we change this PR to use podInfo here?

We cannot do it in this PR. The FIFO scheduling queue does not use podInfo. The changes you made in the factory.go apply to FIFO queue only.
In a follow-up PR, we can remove the FIFO queue and switch to podInfo and delete this podLastUpdateTime map.

Many of the functions here are thread safe, but this one is not. It is left to the caller to acquire the lock. I guess the reason is that CleanupPodsCompletesBackingoff uses this function. Please create a thread safe version of this function which is public (starts with a capital letter) and use the new function in other modules.

Updated

the body of this function is non-trivial. Please use defer pbm.lock.Unlock() in the beginning of the function.

Updated