From 239d92595caa5025c01442818b9b36d37b6e13ae Mon Sep 17 00:00:00 2001 From: JenTing Hsiao Date: Tue, 14 Jun 2022 03:26:44 +0000 Subject: [PATCH 1/2] [ws-manager] fix the possibility the volume restore time is incorrect We might fall into the exponential backoff function, meaning that the volume restore time would be calculated again even if we had already calculated the volume restore time. Signed-off-by: JenTing Hsiao --- components/ws-manager/pkg/manager/manager.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/components/ws-manager/pkg/manager/manager.go b/components/ws-manager/pkg/manager/manager.go index 4d63fbb2a6ca66..b50fef705a96c4 100644 --- a/components/ws-manager/pkg/manager/manager.go +++ b/components/ws-manager/pkg/manager/manager.go @@ -215,9 +215,9 @@ func (m *Manager) StartWorkspace(ctx context.Context, req *api.StartWorkspaceReq span.LogKV("event", "pod description created") var ( - createPVC bool - pvc *corev1.PersistentVolumeClaim - volumeRestoreTime time.Time + createPVC bool + pvc *corev1.PersistentVolumeClaim + startTime, endTime time.Time ) for _, feature := range startContext.Request.Spec.FeatureFlags { if feature == api.WorkspaceFeatureFlag_PERSISTENT_VOLUME_CLAIM { @@ -235,7 +235,7 @@ func (m *Manager) StartWorkspace(ctx context.Context, req *api.StartWorkspaceReq if err != nil && !k8serr.IsAlreadyExists(err) { return nil, xerrors.Errorf("cannot create pvc object for workspace pod: %w", err) } - volumeRestoreTime = time.Now() + startTime = time.Now() } // create the Pod in the cluster and wait until is scheduled @@ -276,8 +276,9 @@ func (m *Manager) StartWorkspace(ctx context.Context, req *api.StartWorkspaceReq hist, err := m.metrics.volumeRestoreTimeHistVec.GetMetricWithLabelValues(wsType, req.Spec.Class) if err != nil { log.WithError(err).WithField("type", wsType).Warn("cannot get volume restore time histogram metric") - } else { - hist.Observe(time.Since(volumeRestoreTime).Seconds()) + } else if endTime.IsZero() { + endTime = time.Now() + hist.Observe(endTime.Sub(startTime).Seconds()) } } From 741c085424ca0d5821e7b2f9f4b75bf6ebce191c Mon Sep 17 00:00:00 2001 From: JenTing Hsiao Date: Tue, 14 Jun 2022 03:45:22 +0000 Subject: [PATCH 2/2] [ws-manager] Only calculate the volume restore time if it's restoring from the VolumeSnapshot Signed-off-by: JenTing Hsiao --- components/ws-manager/pkg/manager/manager.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/components/ws-manager/pkg/manager/manager.go b/components/ws-manager/pkg/manager/manager.go index b50fef705a96c4..97069c6cbdce60 100644 --- a/components/ws-manager/pkg/manager/manager.go +++ b/components/ws-manager/pkg/manager/manager.go @@ -217,7 +217,7 @@ func (m *Manager) StartWorkspace(ctx context.Context, req *api.StartWorkspaceReq var ( createPVC bool pvc *corev1.PersistentVolumeClaim - startTime, endTime time.Time + startTime, endTime time.Time // the start time and end time of PVC restoring from VolumeSnapshot ) for _, feature := range startContext.Request.Spec.FeatureFlags { if feature == api.WorkspaceFeatureFlag_PERSISTENT_VOLUME_CLAIM { @@ -235,7 +235,10 @@ func (m *Manager) StartWorkspace(ctx context.Context, req *api.StartWorkspaceReq if err != nil && !k8serr.IsAlreadyExists(err) { return nil, xerrors.Errorf("cannot create pvc object for workspace pod: %w", err) } - startTime = time.Now() + // we only calculate the time that PVC restoring from VolumeSnapshot + if startContext.VolumeSnapshot != nil && startContext.VolumeSnapshot.VolumeSnapshotName != "" { + startTime = time.Now() + } } // create the Pod in the cluster and wait until is scheduled @@ -266,7 +269,8 @@ func (m *Manager) StartWorkspace(ctx context.Context, req *api.StartWorkspaceReq return false, err } - if createPVC { + // we only calculate the time that PVC restoring from VolumeSnapshot + if createPVC && startContext.VolumeSnapshot != nil && startContext.VolumeSnapshot.VolumeSnapshotName != "" { err = wait.PollWithContext(ctx, 100*time.Millisecond, time.Minute, pvcRunning(m.Clientset, pvc.Name, pvc.Namespace)) if err != nil { return false, nil