replication: reduce the reqeue time for GetReplicationInfo

Reduce the schedule time by half to get the
latest update and also to avoid the
inconsistency between the last sync time in
the VR and the Storage system.

The user can see updates for RPO that are not
stuck in a bad schedule race i.e VR checks
and finds sync time as t-5m and just after
that storage system updates it to t+x.
If we checked every 1/2 of schedule we will
update it to t+x in t+s/2

Signed-off-by: Madhu Rajanna <[email protected]>

controllers/replication.storage/volumereplication_controller.go

@@ -392,20 +392,22 @@ func (r *VolumeReplicationReconciler) Reconcile(ctx context.Context, req ctrl.Re
 	logger.Info(msg)
 
 	if requeueForInfo {
-		scheduleTime := getScheduleTime(parameters, logger)
+		reconcileInternal := getInfoReconcileInterval(parameters, logger)
 		return ctrl.Result{
 			Requeue:      true,
-			RequeueAfter: scheduleTime,
+			RequeueAfter: reconcileInternal,
 		}, nil
 	}
 
 	return ctrl.Result{}, nil
 }
 
-// getScheduleTime takes parameters and returns the scheduling interval
-// after converting it to time.Duration. If the schedulingInterval is empty
-// or there is error parsing, it is set to the default value.
-func getScheduleTime(parameters map[string]string, logger logr.Logger) time.Duration {
+// getInfoReconcileInterval takes parameters and returns the half of the scheduling
+// interval after converting it to time.Duration. The interval is converted
+// into half to keep the LastSyncTime and Storage LastSyncTime to be in sync.
+// If the schedulingInterval is empty or there is error parsing, it is set to
+// the default value.
+func getInfoReconcileInterval(parameters map[string]string, logger logr.Logger) time.Duration {
 	// the schedulingInterval looks like below, which is the part of volumereplicationclass
 	// and is an optional parameter.
 	// ```parameters:
@@ -421,7 +423,17 @@ func getScheduleTime(parameters map[string]string, logger logr.Logger) time.Dura
 		logger.Error(err, "failed to parse time: %v", rawScheduleTime)
 		return defaultScheduleTime
 	}
-	return scheduleTime
+	// Return schedule internal to avoid frequent reconcile if the
+	// schedulingInterval is less than 2 minutes.
+	if scheduleTime < 2*time.Minute {
+		logger.Info("scheduling interval is less than 2 minutes, not decreasing it to half")
+		return scheduleTime
+	}
+
+	// Reduce the schedule time by half to get the latest update and also to
+	// avoid the inconsistency between the last sync time in the VR and the
+	// Storage system.
+	return scheduleTime / 2
 }
 
 func (r *VolumeReplicationReconciler) getReplicationClient(driverName string) (grpcClient.VolumeReplication, error) {

controllers/replication.storage/volumereplication_test.go

@@ -25,7 +25,6 @@ import (
 
 func TestGetScheduledTime(t *testing.T) {
 	t.Parallel()
-	td, _ := time.ParseDuration("1m")
 	const defaultScheduleTime = time.Hour
 	logger := testr.New(t)
 	testcases := []struct {
@@ -37,7 +36,7 @@ func TestGetScheduledTime(t *testing.T) {
 				"replication.storage.openshift.io/replication-secret-name": "rook-csi-rbd-provisioner",
 				"schedulingInterval": "1m",
 			},
-			time: td,
+			time: time.Minute,
 		},
 		{
 			parameters: map[string]string{
@@ -61,12 +60,30 @@ func TestGetScheduledTime(t *testing.T) {
 			},
 			time: defaultScheduleTime,
 		},
+		{
+			parameters: map[string]string{
+				"schedulingInterval": "10s",
+			},
+			time: 10 * time.Second,
+		},
+		{
+			parameters: map[string]string{
+				"schedulingInterval": "4m",
+			},
+			time: 2 * time.Minute,
+		},
+		{
+			parameters: map[string]string{
+				"schedulingInterval": "1h",
+			},
+			time: 30 * time.Minute,
+		},
 	}
 	for _, tt := range testcases {
 		newtt := tt
 		t.Run("", func(t *testing.T) {
 			t.Parallel()
-			if got := getScheduleTime(newtt.parameters, logger); got != newtt.time {
+			if got := getInfoReconcileInterval(newtt.parameters, logger); got != newtt.time {
 				t.Errorf("GetSchedluedTime() = %v, want %v", got, newtt.time)
 			}
 		})

docs/volumereplicationclass.md

@@ -10,8 +10,8 @@
 
 + `replication.storage.openshift.io/replication-secret-name`
 + `replication.storage.openshift.io/replication-secret-namespace`
-  
-``` yaml  
+
+``` yaml
 apiVersion: replication.storage.openshift.io/v1alpha1
 kind: VolumeReplicationClass
 metadata:
@@ -21,5 +21,8 @@ spec:
   parameters:
     replication.storage.openshift.io/replication-secret-name: secret-name
     replication.storage.openshift.io/replication-secret-namespace: secret-namespace
+    # schedulingInterval is a vendor specific parameter. It is used to set the
+    # replication scheduling interval for storage volumes that are replication
+    # enabled using related VolumeReplication resource
     schedulingInterval: 1m
 ```