feat: cache resolved git/helm repository revisions (argoproj#6102)

* feat: cache resolved git/helm repository revisions Signed-off-by: Alexander Matyushentsev <[email protected]>
dajudge · May 19, 2021 · 9b32e01 · 9b32e01
1 parent 44dccdb
commit 9b32e01
Show file tree

Hide file tree

Showing 32 changed files with 538 additions and 281 deletions.
diff --git a/cmd/argocd-application-controller/commands/argocd_application_controller.go b/cmd/argocd-application-controller/commands/argocd_application_controller.go
@@ -137,7 +137,7 @@ func NewCommand() *cobra.Command {
  }
 
  clientConfig = cli.AddKubectlFlagsToCmd(&command)
- command.Flags().Int64Var(&appResyncPeriod, "app-resync", defaultAppResyncPeriod, "Time period in seconds for application resync.")
+ command.Flags().Int64Var(&appResyncPeriod, "app-resync", int64(env.ParseDurationFromEnv("ARGOCD_RECONCILIATION_TIMEOUT", defaultAppResyncPeriod, 0, math.MaxInt32).Seconds()), "Time period in seconds for application resync.")
  command.Flags().StringVar(&repoServerAddress, "repo-server", common.DefaultRepoServerAddr, "Repo server address.")
  command.Flags().IntVar(&repoServerTimeoutSeconds, "repo-server-timeout-seconds", 60, "Repo server RPC call timeout seconds.")
  command.Flags().IntVar(&statusProcessors, "status-processors", 1, "Number of application status processors")

diff --git a/cmd/argocd-util/commands/app.go b/cmd/argocd-util/commands/app.go
@@ -390,7 +390,7 @@ func reconcileApplications(
  return nil, err
  }
 
- res := appStateManager.CompareAppState(&app, proj, app.Spec.Source.TargetRevision, app.Spec.Source, false, nil)
+ res := appStateManager.CompareAppState(&app, proj, app.Spec.Source.TargetRevision, app.Spec.Source, false, false, nil)
  items = append(items, appReconcileResult{
  Name: app.Name,
  Conditions: app.Status.Conditions,

diff --git a/controller/appcontroller.go b/controller/appcontroller.go
@@ -66,6 +66,8 @@ const (
 type CompareWith int
 
 const (
+ // Compare live application state against state defined in latest git revision with no resolved revision caching.
+ CompareWithLatestForceResolve CompareWith = 3
  // Compare live application state against state defined in latest git revision.
  CompareWithLatest CompareWith = 2
  // Compare live application state against state defined using revision of most recent comparison.
@@ -1199,7 +1201,9 @@ func (ctrl *ApplicationController) processAppRefreshQueueItem() (processNext boo
  }
 
  now := metav1.Now()
- compareResult := ctrl.appStateManager.CompareAppState(app, project, revision, app.Spec.Source, refreshType == appv1.RefreshTypeHard, localManifests)
+ compareResult := ctrl.appStateManager.CompareAppState(app, project, revision, app.Spec.Source,
+ refreshType == appv1.RefreshTypeHard,
+ comparisonLevel == CompareWithLatestForceResolve, localManifests)
  for k, v := range compareResult.timings {
  logCtx = logCtx.WithField(k, v.Milliseconds())
  }
@@ -1230,7 +1234,7 @@ func (ctrl *ApplicationController) processAppRefreshQueueItem() (processNext boo
  logCtx.Info("Sync prevented by sync window")
  }
 
- if app.Status.ReconciledAt == nil || comparisonLevel == CompareWithLatest {
+ if app.Status.ReconciledAt == nil || comparisonLevel >= CompareWithLatest {
  app.Status.ReconciledAt = &now
  }
  app.Status.Sync = *compareResult.syncStatus
@@ -1260,9 +1264,13 @@ func (ctrl *ApplicationController) needRefreshAppStatus(app *appv1.Application,
  expired := app.Status.ReconciledAt == nil || app.Status.ReconciledAt.Add(statusRefreshTimeout).Before(time.Now().UTC())
 
  if requestedType, ok := app.IsRefreshRequested(); ok {
+ compareWith = CompareWithLatestForceResolve
  // user requested app refresh.
  refreshType = requestedType
  reason = fmt.Sprintf("%s refresh requested", refreshType)
+ } else if !app.Spec.Source.Equals(app.Status.Sync.ComparedTo.Source) {
+ reason = "spec.source differs"
+ compareWith = CompareWithLatestForceResolve
  } else if expired {
  // The commented line below mysteriously crashes if app.Status.ReconciledAt is nil
  // reason = fmt.Sprintf("comparison expired. reconciledAt: %v, expiry: %v", app.Status.ReconciledAt, statusRefreshTimeout)
@@ -1272,8 +1280,6 @@ func (ctrl *ApplicationController) needRefreshAppStatus(app *appv1.Application,
  reconciledAtStr = app.Status.ReconciledAt.String()
  }
  reason = fmt.Sprintf("comparison expired. reconciledAt: %v, expiry: %v", reconciledAtStr, statusRefreshTimeout)
- } else if !app.Spec.Source.Equals(app.Status.Sync.ComparedTo.Source) {
- reason = "spec.source differs"
  } else if !app.Spec.Destination.Equals(app.Status.Sync.ComparedTo.Destination) {
  reason = "spec.destination differs"
  } else if requested, level := ctrl.isRefreshRequested(app.Name); requested {

diff --git a/controller/appcontroller_test.go b/controller/appcontroller_test.go
@@ -855,7 +855,7 @@ func TestNeedRefreshAppStatus(t *testing.T) {
  needRefresh, refreshType, compareWith = ctrl.needRefreshAppStatus(app, 1*time.Hour)
  assert.True(t, needRefresh)
  assert.Equal(t, argoappv1.RefreshTypeNormal, refreshType)
- assert.Equal(t, CompareWithLatest, compareWith)
+ assert.Equal(t, CompareWithLatestForceResolve, compareWith)
 
  {
  // refresh app using the 'latest' level if comparison expired
@@ -866,7 +866,7 @@ func TestNeedRefreshAppStatus(t *testing.T) {
  needRefresh, refreshType, compareWith = ctrl.needRefreshAppStatus(app, 1*time.Minute)
  assert.True(t, needRefresh)
  assert.Equal(t, argoappv1.RefreshTypeNormal, refreshType)
- assert.Equal(t, CompareWithLatest, compareWith)
+ assert.Equal(t, CompareWithLatestForceResolve, compareWith)
  }
 
  {
@@ -880,7 +880,7 @@ func TestNeedRefreshAppStatus(t *testing.T) {
  needRefresh, refreshType, compareWith = ctrl.needRefreshAppStatus(app, 1*time.Hour)
  assert.True(t, needRefresh)
  assert.Equal(t, argoappv1.RefreshTypeHard, refreshType)
- assert.Equal(t, CompareWithLatest, compareWith)
+ assert.Equal(t, CompareWithLatestForceResolve, compareWith)
  }
 
  {
@@ -898,7 +898,7 @@ func TestNeedRefreshAppStatus(t *testing.T) {
  needRefresh, refreshType, compareWith = ctrl.needRefreshAppStatus(app, 1*time.Hour)
  assert.True(t, needRefresh)
  assert.Equal(t, argoappv1.RefreshTypeNormal, refreshType)
- assert.Equal(t, CompareWithLatest, compareWith)
+ assert.Equal(t, CompareWithLatestForceResolve, compareWith)
  }
 }
 

diff --git a/controller/state.go b/controller/state.go
@@ -75,7 +75,7 @@ func GetLiveObjsForApplicationHealth(resources []managedResource, statuses []app
 
 // AppStateManager defines methods which allow to compare application spec and actual application state.
 type AppStateManager interface {
- CompareAppState(app *v1alpha1.Application, project *appv1.AppProject, revision string, source v1alpha1.ApplicationSource, noCache bool, localObjects []string) *comparisonResult
+ CompareAppState(app *v1alpha1.Application, project *appv1.AppProject, revision string, source v1alpha1.ApplicationSource, noCache bool, noRevisionCache bool, localObjects []string) *comparisonResult
  SyncAppState(app *v1alpha1.Application, state *v1alpha1.OperationState)
 }
 
@@ -115,7 +115,7 @@ type appStateManager struct {
  statusRefreshTimeout time.Duration
 }
 
-func (m *appStateManager) getRepoObjs(app *v1alpha1.Application, source v1alpha1.ApplicationSource, appLabelKey, revision string, noCache, verifySignature bool, proj *v1alpha1.AppProject) ([]*unstructured.Unstructured, *apiclient.ManifestResponse, error) {
+func (m *appStateManager) getRepoObjs(app *v1alpha1.Application, source v1alpha1.ApplicationSource, appLabelKey, revision string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject) ([]*unstructured.Unstructured, *apiclient.ManifestResponse, error) {
  ts := stats.NewTimingStats()
  helmRepos, err := m.db.ListHelmRepositories(context.Background())
  if err != nil {
@@ -178,6 +178,7 @@ func (m *appStateManager) getRepoObjs(app *v1alpha1.Application, source v1alpha1
  Repos: permittedHelmRepos,
  Revision: revision,
  NoCache: noCache,
+ NoRevisionCache: noRevisionCache,
  AppLabelKey: appLabelKey,
  AppName: app.Name,
  Namespace: app.Spec.Destination.Namespace,
@@ -375,7 +376,7 @@ func (m *appStateManager) diffArrayCached(configArray []*unstructured.Unstructur
 // CompareAppState compares application git state to the live app state, using the specified
 // revision and supplied source. If revision or overrides are empty, then compares against
 // revision and overrides in the app spec.
-func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *appv1.AppProject, revision string, source v1alpha1.ApplicationSource, noCache bool, localManifests []string) *comparisonResult {
+func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *appv1.AppProject, revision string, source v1alpha1.ApplicationSource, noCache bool, noRevisionCache bool, localManifests []string) *comparisonResult {
  ts := stats.NewTimingStats()
  appLabelKey, resourceOverrides, diffNormalizer, resFilter, err := m.getComparisonSettings(app)
  ts.AddCheckpoint("settings_ms")
@@ -409,7 +410,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *ap
  now := metav1.Now()
 
  if len(localManifests) == 0 {
- targetObjs, manifestInfo, err = m.getRepoObjs(app, source, appLabelKey, revision, noCache, verifySignature, project)
+ targetObjs, manifestInfo, err = m.getRepoObjs(app, source, appLabelKey, revision, noCache, noRevisionCache, verifySignature, project)
  if err != nil {
  targetObjs = make([]*unstructured.Unstructured, 0)
  conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error(), LastTransitionTime: &now})

diff --git a/controller/state_test.go b/controller/state_test.go
@@ -36,7 +36,7 @@ func TestCompareAppStateEmpty(t *testing.T) {
  managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured),
  }
  ctrl := newFakeController(&data)
- compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, false, nil)
  assert.NotNil(t, compRes)
  assert.NotNil(t, compRes.syncStatus)
  assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status)
@@ -59,7 +59,7 @@ func TestCompareAppStateMissing(t *testing.T) {
  managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured),
  }
  ctrl := newFakeController(&data)
- compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, false, nil)
  assert.NotNil(t, compRes)
  assert.NotNil(t, compRes.syncStatus)
  assert.Equal(t, argoappv1.SyncStatusCodeOutOfSync, compRes.syncStatus.Status)
@@ -86,7 +86,7 @@ func TestCompareAppStateExtra(t *testing.T) {
  },
  }
  ctrl := newFakeController(&data)
- compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, false, nil)
  assert.NotNil(t, compRes)
  assert.Equal(t, argoappv1.SyncStatusCodeOutOfSync, compRes.syncStatus.Status)
  assert.Equal(t, 1, len(compRes.resources))
@@ -112,7 +112,7 @@ func TestCompareAppStateHook(t *testing.T) {
  managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured),
  }
  ctrl := newFakeController(&data)
- compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, false, nil)
  assert.NotNil(t, compRes)
  assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status)
  assert.Equal(t, 0, len(compRes.resources))
@@ -139,7 +139,7 @@ func TestCompareAppStateSkipHook(t *testing.T) {
  managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured),
  }
  ctrl := newFakeController(&data)
- compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, false, nil)
  assert.NotNil(t, compRes)
  assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status)
  assert.Equal(t, 1, len(compRes.resources))
@@ -165,7 +165,7 @@ func TestCompareAppStateCompareOptionIgnoreExtraneous(t *testing.T) {
  }
  ctrl := newFakeController(&data)
 
- compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, false, nil)
 
  assert.NotNil(t, compRes)
  assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status)
@@ -193,7 +193,7 @@ func TestCompareAppStateExtraHook(t *testing.T) {
  },
  }
  ctrl := newFakeController(&data)
- compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, false, nil)
 
  assert.NotNil(t, compRes)
  assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status)
@@ -236,7 +236,7 @@ func TestCompareAppStateDuplicatedNamespacedResources(t *testing.T) {
  },
  }
  ctrl := newFakeController(&data)
- compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, false, nil)
 
  assert.NotNil(t, compRes)
  assert.Equal(t, 1, len(app.Status.Conditions))
@@ -287,7 +287,7 @@ func TestSetHealth(t *testing.T) {
  },
  })
 
- compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, false, nil)
 
  assert.Equal(t, compRes.healthStatus.Status, health.HealthStatusHealthy)
 }
@@ -319,7 +319,7 @@ func TestSetHealthSelfReferencedApp(t *testing.T) {
  },
  })
 
- compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, false, nil)
 
  assert.Equal(t, compRes.healthStatus.Status, health.HealthStatusHealthy)
 }
@@ -389,7 +389,7 @@ func TestReturnUnknownComparisonStateOnSettingLoadError(t *testing.T) {
  },
  })
 
- compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, false, nil)
 
  assert.Equal(t, health.HealthStatusUnknown, compRes.healthStatus.Status)
  assert.Equal(t, argoappv1.SyncStatusCodeUnknown, compRes.syncStatus.Status)
@@ -525,7 +525,7 @@ func TestSignedResponseNoSignatureRequired(t *testing.T) {
  managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured),
  }
  ctrl := newFakeController(&data)
- compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, false, nil)
  assert.NotNil(t, compRes)
  assert.NotNil(t, compRes.syncStatus)
  assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status)
@@ -547,7 +547,7 @@ func TestSignedResponseNoSignatureRequired(t *testing.T) {
  managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured),
  }
  ctrl := newFakeController(&data)
- compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, "", app.Spec.Source, false, false, nil)
  assert.NotNil(t, compRes)
  assert.NotNil(t, compRes.syncStatus)
  assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status)
@@ -576,7 +576,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) {
  managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured),
  }
  ctrl := newFakeController(&data)
- compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, "", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, "", app.Spec.Source, false, false, nil)
  assert.NotNil(t, compRes)
  assert.NotNil(t, compRes.syncStatus)
  assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status)
@@ -598,7 +598,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) {
  managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured),
  }
  ctrl := newFakeController(&data)
- compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, "abc123", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, "abc123", app.Spec.Source, false, false, nil)
  assert.NotNil(t, compRes)
  assert.NotNil(t, compRes.syncStatus)
  assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status)
@@ -620,7 +620,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) {
  managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured),
  }
  ctrl := newFakeController(&data)
- compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, "abc123", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, "abc123", app.Spec.Source, false, false, nil)
  assert.NotNil(t, compRes)
  assert.NotNil(t, compRes.syncStatus)
  assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status)
@@ -642,7 +642,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) {
  managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured),
  }
  ctrl := newFakeController(&data)
- compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, "abc123", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, "abc123", app.Spec.Source, false, false, nil)
  assert.NotNil(t, compRes)
  assert.NotNil(t, compRes.syncStatus)
  assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status)
@@ -667,7 +667,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) {
  ctrl := newFakeController(&data)
  testProj := signedProj
  testProj.Spec.SignatureKeys[0].KeyID = "4AEE18F83AFDEB24"
- compRes := ctrl.appStateManager.CompareAppState(app, &testProj, "abc123", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &testProj, "abc123", app.Spec.Source, false, false, nil)
  assert.NotNil(t, compRes)
  assert.NotNil(t, compRes.syncStatus)
  assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status)
@@ -692,7 +692,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) {
  // it doesn't matter for our test whether local manifests are valid
  localManifests := []string{"foobar"}
  ctrl := newFakeController(&data)
- compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, "abc123", app.Spec.Source, false, localManifests)
+ compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, "abc123", app.Spec.Source, false, false, localManifests)
  assert.NotNil(t, compRes)
  assert.NotNil(t, compRes.syncStatus)
  assert.Equal(t, argoappv1.SyncStatusCodeUnknown, compRes.syncStatus.Status)
@@ -717,7 +717,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) {
  managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured),
  }
  ctrl := newFakeController(&data)
- compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, "abc123", app.Spec.Source, false, nil)
+ compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, "abc123", app.Spec.Source, false, false, nil)
  assert.NotNil(t, compRes)
  assert.NotNil(t, compRes.syncStatus)
  assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status)
@@ -742,7 +742,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) {
  // it doesn't matter for our test whether local manifests are valid
  localManifests := []string{""}
  ctrl := newFakeController(&data)
- compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, "abc123", app.Spec.Source, false, localManifests)
+ compRes := ctrl.appStateManager.CompareAppState(app, &signedProj, "abc123", app.Spec.Source, false, false, localManifests)
  assert.NotNil(t, compRes)
  assert.NotNil(t, compRes.syncStatus)
  assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status)

diff --git a/controller/sync.go b/controller/sync.go
@@ -85,7 +85,7 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
  return
  }
 
- compareResult := m.CompareAppState(app, proj, revision, source, false, syncOp.Manifests)
+ compareResult := m.CompareAppState(app, proj, revision, source, false, true, syncOp.Manifests)
  // We now have a concrete commit SHA. Save this in the sync result revision so that we remember
  // what we should be syncing to when resuming operations.
  syncRes.Revision = compareResult.syncStatus.Revision