Skip to content

Commit 4479e24

Browse files
authored
Use diff prop collectors for two WaitForUpdates calls (#3182)
This patch updates the storagepool listner and listview used to watch tasks so the two, distinct calls to WaitForUpdatesEx use distinct PropertyCollectors. This is necessary because the vSphere API docs do state that WaitForUpdates and WaitForUpdatesEx do not support concurrent access to the same PropertyCollector. Without this patch, when running at scale, CSI can timeout when waiting for updates because the session's default PropertyCollector ends up in a blocking state on the backend.
1 parent 98404f4 commit 4479e24

File tree

2 files changed

+26
-3
lines changed

2 files changed

+26
-3
lines changed

pkg/common/cns-lib/volume/listview.go

+12-1
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,12 @@ func (l *ListViewImpl) listenToTaskUpdates() {
339339

340340
log.Info("Starting listening for task updates...")
341341
log.Infof("waitForUpdatesContext %v", l.waitForUpdatesContext)
342-
pc := property.DefaultCollector(l.virtualCenter.Client.Client)
342+
pc, pcErr := property.DefaultCollector(l.virtualCenter.Client.Client).Create(l.ctx)
343+
if pcErr != nil {
344+
log.Errorf("failed to create PropertyCollector for WaitForUpdatesEx for ListView. error: %+v", pcErr)
345+
l.mu.Unlock()
346+
continue
347+
}
343348
l.isReady = true
344349
log.Infof("listview ready state is %v", l.isReady)
345350
l.mu.Unlock()
@@ -357,6 +362,12 @@ func (l *ListViewImpl) listenToTaskUpdates() {
357362
// we only want this true while running the unit tests so the test can finish
358363
return l.shouldStopListening
359364
})
365+
366+
// Attempt to clean up the property collector using a new context to
367+
// ensure it goes through. This call *might* fail if the session's
368+
// auth has expired, but it is worth trying.
369+
_ = pc.Destroy(context.Background())
370+
360371
// if property collector returns any errors,
361372
// we want to immediately return a fault for all the pending tasks in the map
362373
// note: this is not a task error but an error from the vc

pkg/syncer/storagepool/listener.go

+14-2
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,14 @@ func initListener(ctx context.Context, scWatchCntlr *StorageClassWatch,
118118

119119
for {
120120
log.Infof("Starting property collector...")
121-
p := property.DefaultCollector(spController.vc.Client.Client)
122-
err := property.WaitForUpdatesEx(ctx, p, filter, func(updates []types.ObjectUpdate) bool {
121+
122+
pc, pcErr := property.DefaultCollector(spController.vc.Client.Client).Create(ctx)
123+
if err != nil {
124+
log.Errorf("Not able to create property collector. Restarting property collector. error: %+v", pcErr)
125+
return
126+
}
127+
128+
err := property.WaitForUpdatesEx(ctx, pc, filter, func(updates []types.ObjectUpdate) bool {
123129
ctx := logger.NewContextWithLogger(ctx)
124130
log = logger.GetLogger(ctx)
125131
log.Debugf("Got %d property collector update(s)", len(updates))
@@ -159,6 +165,12 @@ func initListener(ctx context.Context, scWatchCntlr *StorageClassWatch,
159165
log.Debugf("Done processing %d property collector update(s)", len(updates))
160166
return false
161167
})
168+
169+
// Attempt to clean up the property collector using a new context to
170+
// ensure it goes through. This call *might* fail if the session's
171+
// auth has expired, but it is worth trying.
172+
_ = pc.Destroy(context.Background())
173+
162174
if err != nil {
163175
log.Infof("Property collector session needs to be reestablished due to err: %v", err)
164176
err = spController.vc.Connect(ctx)

0 commit comments

Comments
 (0)