SOR: re-enable ES client retry mechanism (#184761)

## Summary

We recently "discovered" that ES calls performed from the SOR were not
having the default retry mechanism used by the ES client.

This is caused by the way we construct the ES client wrapper used for
the SOR, that forces the `maxRetries` option to `0`


8911bfabb0/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/repository_es_client.ts (L38-L40)

I initially tried to fully get rid of `retryCallCluster` in
https://github.com/elastic/kibana/pull/184658, as we assumed that
re-enabling the retry mechanism would be sufficient, but this was
causing flakiness in test suites (uncaught rejections in plugins not
properly stopping after ES and Kibana are shut down, with
`NoLivingConnectionsError` errors...)

So instead, I just re-enabled the ES client native mechanism by removing
the `maxRetries` override.
This commit is contained in:
Pierre Gayvallet 2024-06-05 10:34:20 +02:00 committed by GitHub
parent 958072af0c
commit 374282ded0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 13 additions and 17 deletions

View file

@ -78,7 +78,7 @@ describe('SavedObjectsRepository Spaces Extension', () => {
const documentMigrator = createDocumentMigrator(registry);
// const currentSpace = 'foo-namespace';
const defaultOptions = { ignore: [404], maxRetries: 0, meta: true }; // These are just the hard-coded options passed in via the repo
const defaultOptions = { ignore: [404], meta: true }; // These are just the hard-coded options passed in via the repo
const instantiateRepository = () => {
const allTypes = registry.getAllTypes().map((type) => type.name);
@ -283,7 +283,7 @@ describe('SavedObjectsRepository Spaces Extension', () => {
: { type: CUSTOM_INDEX_TYPE, customIndex: attributes }
),
}),
{ maxRetries: 0, meta: true }
{ meta: true }
);
});
});
@ -316,7 +316,7 @@ describe('SavedObjectsRepository Spaces Extension', () => {
expect.objectContaining({
id: expect.stringMatching(regex),
}),
{ ignore: [404], maxRetries: 0, meta: true }
{ ignore: [404], meta: true }
);
});
});
@ -391,7 +391,7 @@ describe('SavedObjectsRepository Spaces Extension', () => {
]),
}),
}),
{ ignore: [404], maxRetries: 0, meta: true }
{ ignore: [404], meta: true }
);
});
});
@ -582,7 +582,7 @@ describe('SavedObjectsRepository Spaces Extension', () => {
]),
}),
}),
{ ignore: [404], maxRetries: 0, meta: true }
{ ignore: [404], meta: true }
);
});
@ -652,7 +652,7 @@ describe('SavedObjectsRepository Spaces Extension', () => {
}),
]),
}),
{ maxRetries: 0 }
{}
);
});
});
@ -717,7 +717,7 @@ describe('SavedObjectsRepository Spaces Extension', () => {
}),
]),
}),
{ maxRetries: 0 }
{}
);
});
});
@ -869,7 +869,7 @@ describe('SavedObjectsRepository Spaces Extension', () => {
}),
]),
}),
{ maxRetries: 0 }
{}
);
});
});

View file

@ -33,13 +33,11 @@ describe('RepositoryEsClient', () => {
expect(retryCallClusterMock).toHaveBeenCalledTimes(1);
});
it('sets maxRetries: 0 to delegate retry logic to retryCallCluster', async () => {
it('keeps call options unchanged', async () => {
expect(repositoryClient.bulk).toStrictEqual(expect.any(Function));
await repositoryClient.bulk({ body: [] });
expect(client.bulk).toHaveBeenCalledWith(
expect.any(Object),
expect.objectContaining({ maxRetries: 0 })
);
const options = { maxRetries: 12 };
await repositoryClient.bulk({ body: [] }, options);
expect(client.bulk).toHaveBeenCalledWith(expect.any(Object), options);
});
it('transform elasticsearch errors into saved objects errors', async () => {

View file

@ -35,9 +35,7 @@ export function createRepositoryEsClient(client: ElasticsearchClient): Repositor
Object.defineProperty(acc, key, {
value: async (params?: unknown, options?: TransportRequestOptions) => {
try {
return await retryCallCluster(() =>
(client[key] as Function)(params, { maxRetries: 0, ...options })
);
return await retryCallCluster(() => (client[key] as Function)(params, options ?? {}));
} catch (e) {
// retry failures are caught here, as are 404's that aren't ignored (e.g update calls)
throw decorateEsError(e);