Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: warning log if primary beacon node is unhealthy #6921

Merged
merged 5 commits into from
Jul 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/api/src/beacon/client/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type ClientModules = HttpClientModules & {
httpClient?: IHttpClient;
};

export type ApiClient = {[K in keyof Endpoints]: ApiClientMethods<Endpoints[K]>};
export type ApiClient = {[K in keyof Endpoints]: ApiClientMethods<Endpoints[K]>} & {httpClient: IHttpClient};

/**
* REST HTTP client for all routes
Expand All @@ -42,5 +42,6 @@ export function getClient(opts: HttpClientOptions, modules: ClientModules): ApiC
node: node.getClient(config, httpClient),
proof: proof.getClient(config, httpClient),
validator: validator.getClient(config, httpClient),
httpClient,
Copy link
Member Author

@nflaig nflaig Jun 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative to exposing the http client here would be to emit the warning inside the http client itself but that's not ideal either because a) it does not have any context to what it is connected meaning the log message would have to be more generic and b) it's does not have access to the clock, could emit the warning at an arbitrary interval but emitting on validator makes more sense to me.

};
}
5 changes: 3 additions & 2 deletions packages/api/src/utils/client/httpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ export const defaultInit: Required<ExtraRequestInit> = {

export interface IHttpClient {
readonly baseUrl: string;
readonly urlsInits: UrlInitRequired[];
readonly urlsScore: number[];

request<E extends Endpoint>(
definition: RouteDefinitionExtra<E>,
Expand All @@ -71,14 +73,13 @@ export type HttpClientModules = {

export class HttpClient implements IHttpClient {
readonly urlsInits: UrlInitRequired[] = [];
readonly urlsScore: number[];

private readonly signal: null | AbortSignal;
private readonly fetch: typeof fetch;
private readonly metrics: null | Metrics;
private readonly logger: null | Logger;

private readonly urlsScore: number[];

/**
* Cache to keep track of routes per server that do not support SSZ. This cache will only be
* populated if we receive a 415 error response from the server after sending a SSZ request body.
Expand Down
12 changes: 11 additions & 1 deletion packages/beacon-node/test/utils/node/validator.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import tmp from "tmp";
import {vi} from "vitest";
import type {SecretKey} from "@chainsafe/bls/types";
import {LevelDbController} from "@lodestar/db";
import {interopSecretKey} from "@lodestar/state-transition";
Expand Down Expand Up @@ -91,7 +92,7 @@ export async function getAndInitDevValidators({
}

export function getApiFromServerHandlers(api: BeaconApiMethods): ApiClient {
return mapValues(api, (apiModule) =>
const apiClient = mapValues(api, (apiModule) =>
mapValues(apiModule, (api: (args: unknown, context: unknown) => PromiseLike<{data: unknown; meta: unknown}>) => {
return async (args: unknown) => {
try {
Expand All @@ -114,6 +115,15 @@ export function getApiFromServerHandlers(api: BeaconApiMethods): ApiClient {
};
})
) as ApiClient;

apiClient.httpClient = {
baseUrl: "",
request: vi.fn(),
urlsInits: [],
urlsScore: [],
};

return apiClient;
}

export function getNodeApiUrl(node: BeaconNode): string {
Expand Down
15 changes: 15 additions & 0 deletions packages/validator/src/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,21 @@ export class Validator {
this.clock.start(this.controller.signal);
this.chainHeaderTracker.start(this.controller.signal);

// Add notifier to warn user if primary node is unhealthy as there might
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something to consider is a dedicated notifier as we have it in the beacon node, this would allow to emit further information every epoch / slot, for example the statuses of imported validators / active keys, etc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be nice

// not be any errors in the logs due to fallback nodes handling the requests
const {httpClient} = this.api;
if (httpClient.urlsInits.length > 1) {
const primaryNodeUrl = toSafePrintableUrl(httpClient.urlsInits[0].baseUrl);

this.clock?.runEveryEpoch(async () => {
// Only emit warning if URL score is 0 to prevent false positives
// if just a single request fails which might happen due to other reasons
if (httpClient.urlsScore[0] === 0) {
this.logger?.warn("Primary beacon node is unhealthy", {url: primaryNodeUrl});
}
});
}

if (metrics) {
this.db.setMetrics(metrics.db);

Expand Down
18 changes: 15 additions & 3 deletions packages/validator/test/utils/apiStub.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,18 @@
import {vi, Mocked} from "vitest";
import {ApiClientMethods, ApiResponse, Endpoint, Endpoints, HttpStatusCode} from "@lodestar/api";
import {ApiClientMethods, ApiResponse, Endpoint, Endpoints, HttpStatusCode, IHttpClient} from "@lodestar/api";

export function getApiClientStub(): {[K in keyof Endpoints]: Mocked<ApiClientMethods<Endpoints[K]>>} {
type ApiClientStub = {[K in keyof Endpoints]: Mocked<ApiClientMethods<Endpoints[K]>>} & {
httpClient: Mocked<IHttpClient>;
};

const httpClientStub: IHttpClient = {
baseUrl: "",
request: vi.fn(),
urlsInits: [],
urlsScore: [],
};

export function getApiClientStub(): ApiClientStub {
return {
beacon: {
getStateValidators: vi.fn(),
Expand All @@ -25,7 +36,8 @@ export function getApiClientStub(): {[K in keyof Endpoints]: Mocked<ApiClientMet
publishAggregateAndProofs: vi.fn(),
submitBeaconCommitteeSelections: vi.fn(),
},
} as unknown as {[K in keyof Endpoints]: Mocked<ApiClientMethods<Endpoints[K]>>};
httpClient: httpClientStub,
} as unknown as ApiClientStub;
}

export function mockApiResponse<T, M, E extends Endpoint<any, any, any, T, M>>({
Expand Down
Loading