[Alerting] retry internal OCC calls within alertsClient (#77838) (#78688

)

During development of #75553,
some issues came up with the optimistic concurrency control (OCC) we
were using internally within the alertsClient, via the `version`
option/property of the saved object. The referenced PR updates new
fields in the alert from the taskManager task after the alertType
executor runs. In some alertsClient methods, OCC is used to update
the alert which are requested via user requests. And so in some
cases, version conflict errors were coming up when the alert was
updated by task manager, in the middle of one of these methods. Note:
the SIEM function test cases stress test this REALLY well.

In this PR, we wrap all the methods using OCC with a function that
will retry them, a short number of times, with a short delay in
between. If the original method STILL has a conflict error, it
will get thrown after the retry limit.  In practice, this eliminated
the version conflict calls that were occurring with the SIEM tests,
once we started updating the saved object in the executor.

For cases where we know only attributes not contributing to AAD are
being updated, a new function is provided that does a partial update
on just those attributes, making partial updates for those attributes
a bit safer. That will be also used by PR #75553.

x-pack/plugins/alerts/server/alerts_client.test.ts

@@ -1696,14 +1696,22 @@ describe('muteAll()', () => {
         muteAll: false,
       },
       references: [],
+      version: '123',
     });
 
     await alertsClient.muteAll({ id: '1' });
-    expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith('alert', '1', {
-      muteAll: true,
-      mutedInstanceIds: [],
-      updatedBy: 'elastic',
-    });
+    expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith(
+      'alert',
+      '1',
+      {
+        muteAll: true,
+        mutedInstanceIds: [],
+        updatedBy: 'elastic',
+      },
+      {
+        version: '123',
+      }
+    );
   });
 
   describe('authorization', () => {
@@ -1785,11 +1793,18 @@ describe('unmuteAll()', () => {
     });
 
     await alertsClient.unmuteAll({ id: '1' });
-    expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith('alert', '1', {
-      muteAll: false,
-      mutedInstanceIds: [],
-      updatedBy: 'elastic',
-    });
+    expect(unsecuredSavedObjectsClient.update).toHaveBeenCalledWith(
+      'alert',
+      '1',
+      {
+        muteAll: false,
+        mutedInstanceIds: [],
+        updatedBy: 'elastic',
+      },
+      {
+        version: '123',
+      }
+    );
   });
 
   describe('authorization', () => {

x-pack/plugins/alerts/server/alerts_client.ts

@@ -45,6 +45,8 @@ import { parseIsoOrRelativeDate } from './lib/iso_or_relative_date';
 import { alertInstanceSummaryFromEventLog } from './lib/alert_instance_summary_from_event_log';
 import { IEvent } from '../../event_log/server';
 import { parseDuration } from '../common/parse_duration';
+import { retryIfConflicts } from './lib/retry_if_conflicts';
+import { partiallyUpdateAlert } from './saved_objects';
 
 export interface RegistryAlertTypeWithAuth extends RegistryAlertType {
   authorizedConsumers: string[];
@@ -421,6 +423,14 @@ export class AlertsClient {
   }
 
   public async update({ id, data }: UpdateOptions): Promise<PartialAlert> {
+    return await retryIfConflicts(
+      this.logger,
+      `alertsClient.update('${id}')`,
+      async () => await this.updateWithOCC({ id, data })
+    );
+  }
+
+  private async updateWithOCC({ id, data }: UpdateOptions): Promise<PartialAlert> {
     let alertSavedObject: SavedObject<RawAlert>;
 
     try {
@@ -529,7 +539,15 @@ export class AlertsClient {
         };
   }
 
-  public async updateApiKey({ id }: { id: string }) {
+  public async updateApiKey({ id }: { id: string }): Promise<void> {
+    return await retryIfConflicts(
+      this.logger,
+      `alertsClient.updateApiKey('${id}')`,
+      async () => await this.updateApiKeyWithOCC({ id })
+    );
+  }
+
+  private async updateApiKeyWithOCC({ id }: { id: string }) {
     let apiKeyToInvalidate: string | null = null;
     let attributes: RawAlert;
     let version: string | undefined;
@@ -597,7 +615,15 @@ export class AlertsClient {
     }
   }
 
-  public async enable({ id }: { id: string }) {
+  public async enable({ id }: { id: string }): Promise<void> {
+    return await retryIfConflicts(
+      this.logger,
+      `alertsClient.enable('${id}')`,
+      async () => await this.enableWithOCC({ id })
+    );
+  }
+
+  private async enableWithOCC({ id }: { id: string }) {
     let apiKeyToInvalidate: string | null = null;
     let attributes: RawAlert;
     let version: string | undefined;
@@ -658,7 +684,15 @@ export class AlertsClient {
     }
   }
 
-  public async disable({ id }: { id: string }) {
+  public async disable({ id }: { id: string }): Promise<void> {
+    return await retryIfConflicts(
+      this.logger,
+      `alertsClient.disable('${id}')`,
+      async () => await this.disableWithOCC({ id })
+    );
+  }
+
+  private async disableWithOCC({ id }: { id: string }) {
     let apiKeyToInvalidate: string | null = null;
     let attributes: RawAlert;
     let version: string | undefined;
@@ -711,8 +745,19 @@ export class AlertsClient {
     }
   }
 
-  public async muteAll({ id }: { id: string }) {
-    const { attributes } = await this.unsecuredSavedObjectsClient.get<RawAlert>('alert', id);
+  public async muteAll({ id }: { id: string }): Promise<void> {
+    return await retryIfConflicts(
+      this.logger,
+      `alertsClient.muteAll('${id}')`,
+      async () => await this.muteAllWithOCC({ id })
+    );
+  }
+
+  private async muteAllWithOCC({ id }: { id: string }) {
+    const { attributes, version } = await this.unsecuredSavedObjectsClient.get<RawAlert>(
+      'alert',
+      id
+    );
     await this.authorization.ensureAuthorized(
       attributes.alertTypeId,
       attributes.consumer,
@@ -723,19 +768,34 @@ export class AlertsClient {
       await this.actionsAuthorization.ensureAuthorized('execute');
     }
 
-    await this.unsecuredSavedObjectsClient.update(
-      'alert',
+    const updateAttributes = this.updateMeta({
+      muteAll: true,
+      mutedInstanceIds: [],
+      updatedBy: await this.getUserName(),
+    });
+    const updateOptions = { version };
+
+    await partiallyUpdateAlert(
+      this.unsecuredSavedObjectsClient,
       id,
-      this.updateMeta({
-        muteAll: true,
-        mutedInstanceIds: [],
-        updatedBy: await this.getUserName(),
-      })
+      updateAttributes,
+      updateOptions
+    );
+  }
+
+  public async unmuteAll({ id }: { id: string }): Promise<void> {
+    return await retryIfConflicts(
+      this.logger,
+      `alertsClient.unmuteAll('${id}')`,
+      async () => await this.unmuteAllWithOCC({ id })
     );
   }
 
-  public async unmuteAll({ id }: { id: string }) {
-    const { attributes } = await this.unsecuredSavedObjectsClient.get<RawAlert>('alert', id);
+  private async unmuteAllWithOCC({ id }: { id: string }) {
+    const { attributes, version } = await this.unsecuredSavedObjectsClient.get<RawAlert>(
+      'alert',
+      id
+    );
     await this.authorization.ensureAuthorized(
       attributes.alertTypeId,
       attributes.consumer,
@@ -746,18 +806,30 @@ export class AlertsClient {
       await this.actionsAuthorization.ensureAuthorized('execute');
     }
 
-    await this.unsecuredSavedObjectsClient.update(
-      'alert',
+    const updateAttributes = this.updateMeta({
+      muteAll: false,
+      mutedInstanceIds: [],
+      updatedBy: await this.getUserName(),
+    });
+    const updateOptions = { version };
+
+    await partiallyUpdateAlert(
+      this.unsecuredSavedObjectsClient,
       id,
-      this.updateMeta({
-        muteAll: false,
-        mutedInstanceIds: [],
-        updatedBy: await this.getUserName(),
-      })
+      updateAttributes,
+      updateOptions
+    );
+  }
+
+  public async muteInstance({ alertId, alertInstanceId }: MuteOptions): Promise<void> {
+    return await retryIfConflicts(
+      this.logger,
+      `alertsClient.muteInstance('${alertId}')`,
+      async () => await this.muteInstanceWithOCC({ alertId, alertInstanceId })
     );
   }
 
-  public async muteInstance({ alertId, alertInstanceId }: MuteOptions) {
+  private async muteInstanceWithOCC({ alertId, alertInstanceId }: MuteOptions) {
     const { attributes, version } = await this.unsecuredSavedObjectsClient.get<Alert>(
       'alert',
       alertId
@@ -788,7 +860,15 @@ export class AlertsClient {
     }
   }
 
-  public async unmuteInstance({
+  public async unmuteInstance({ alertId, alertInstanceId }: MuteOptions): Promise<void> {
+    return await retryIfConflicts(
+      this.logger,
+      `alertsClient.unmuteInstance('${alertId}')`,
+      async () => await this.unmuteInstanceWithOCC({ alertId, alertInstanceId })
+    );
+  }
+
+  private async unmuteInstanceWithOCC({
     alertId,
     alertInstanceId,
   }: {