pbacsko commented on code in PR #555:
URL: https://github.com/apache/yunikorn-k8shim/pull/555#discussion_r1145815915
##########
pkg/client/clients.go:
##########
@@ -69,19 +71,33 @@ func (c *Clients) GetConf() *conf.SchedulerConf {
return c.conf
}
-func (c *Clients) WaitForSync(interval time.Duration, timeout time.Duration)
error {
- return utils.WaitForCondition(func() bool {
- // cache is re-sync'd when all informers are sync'd
- return c.NodeInformer.Informer().HasSynced() &&
+func (c *Clients) WaitForSync() {
+ timeNow := time.Now()
+ counter := 0
+ for {
+ if c.NodeInformer.Informer().HasSynced() &&
c.PodInformer.Informer().HasSynced() &&
c.PVCInformer.Informer().HasSynced() &&
c.PVInformer.Informer().HasSynced() &&
c.StorageInformer.Informer().HasSynced() &&
c.ConfigMapInformer.Informer().HasSynced() &&
c.NamespaceInformer.Informer().HasSynced() &&
c.PriorityClassInformer.Informer().HasSynced() &&
- (c.AppInformer == nil ||
c.AppInformer.Informer().HasSynced())
- }, interval, timeout)
+ (c.AppInformer == nil ||
c.AppInformer.Informer().HasSynced()) {
+ return
+ }
+ time.Sleep(time.Second)
+ counter++
+ if counter%10 == 0 {
+ if counter > 300 {
+ log.Logger().Warn("Still waiting for informers
to sync, delay seems longer than usual...",
+ zap.Duration("timeElapsed",
time.Since(timeNow).Round(time.Second)))
Review Comment:
Nit: could we rename `timeNow`? It's not changing and it feels weird to
constantly referenced as "now". Like "syncStartTime" or "startTime" sound much
better.
##########
pkg/client/clients.go:
##########
@@ -69,19 +71,33 @@ func (c *Clients) GetConf() *conf.SchedulerConf {
return c.conf
}
-func (c *Clients) WaitForSync(interval time.Duration, timeout time.Duration)
error {
- return utils.WaitForCondition(func() bool {
- // cache is re-sync'd when all informers are sync'd
- return c.NodeInformer.Informer().HasSynced() &&
+func (c *Clients) WaitForSync() {
+ timeNow := time.Now()
+ counter := 0
+ for {
+ if c.NodeInformer.Informer().HasSynced() &&
c.PodInformer.Informer().HasSynced() &&
c.PVCInformer.Informer().HasSynced() &&
c.PVInformer.Informer().HasSynced() &&
c.StorageInformer.Informer().HasSynced() &&
c.ConfigMapInformer.Informer().HasSynced() &&
c.NamespaceInformer.Informer().HasSynced() &&
c.PriorityClassInformer.Informer().HasSynced() &&
- (c.AppInformer == nil ||
c.AppInformer.Informer().HasSynced())
- }, interval, timeout)
+ (c.AppInformer == nil ||
c.AppInformer.Informer().HasSynced()) {
+ return
+ }
+ time.Sleep(time.Second)
+ counter++
+ if counter%10 == 0 {
+ if counter > 300 {
+ log.Logger().Warn("Still waiting for informers
to sync, delay seems longer than usual...",
+ zap.Duration("timeElapsed",
time.Since(timeNow).Round(time.Second)))
Review Comment:
Nit: could we rename `timeNow`? It's not changing and it feels weird to be
referenced as "now". Like "syncStartTime" or "startTime" sound much better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
