Improve logging for backend cluster (de)activation #672

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.

Sign up for GitHub

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

Jump to bottom

Open

amybubu wants to merge 2 commits into trinodb:main from amybubu:activation-status-logging

amybubu commented Apr 15, 2025 •

edited

Loading

Description

As apart of efforts to improve telemetry as described in Issue #649, this PR adds logging for whenever a backend cluster is activated/deactivated.

Testing

built and ran locally
confirmed logs appeared when calling de(activate) via curl call or changing deactivation status via the UI, e.g.:

2025-04-03T17:09:03.588-0700	INFO	http-worker-87	io.trino.gateway.ha.router.HaGatewayManager	Backend cluster trino-3 activation status changed to active=false (previous status: active=true).

confirm error thrown if try to activate/deactivate non-existent backend

java.lang.IllegalStateException: No backend found with name: trino-4, could not activate

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

* Fix some things. ({issue}`issuenumber`)

cla-bot bot added the cla-signed label

xkrogen reviewed

View reviewed changes

Member

xkrogen left a comment

Thanks @amybubu ! Ensuring we have good logging around this has been very helpful to track down the sequence of events for cluster activation/deactivation

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

xkrogen reviewed

View reviewed changes

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Show resolved Hide resolved

andythsu reviewed

View reviewed changes

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

andythsu reviewed

View reviewed changes

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

xkrogen reviewed

View reviewed changes

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

andythsu reviewed

View reviewed changes

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

andythsu reviewed

View reviewed changes

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

Member

andythsu commented Apr 24, 2025

LGTM!

andythsu reviewed

View reviewed changes

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

Member

andythsu commented Apr 24, 2025

pinging @vishalya

ebyhr reviewed

View reviewed changes

Member

ebyhr left a comment

Please squash commits into one and update the PR & commit title.

The current PR title looks misleading. It adds not only logging but also "dao.findFirstByName" call.

ebyhr requested changes

View reviewed changes

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

amybubu force-pushed the activation-status-logging branch from fa232e3 to ead4a1d Compare

May 7, 2025 22:53

amybubu changed the title ~~Add logging when (de)activating backend clusters~~ Modify HaGatewayManager to support informative logging when (de)activating backend clusters

amybubu force-pushed the activation-status-logging branch from ead4a1d to aab05af Compare

May 8, 2025 00:30

ebyhr reviewed

View reviewed changes

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

amybubu force-pushed the activation-status-logging branch from d45c4e4 to 5e3a6ec Compare

May 8, 2025 16:07

ebyhr reviewed

View reviewed changes

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Outdated Show resolved Hide resolved

gateway-ha/src/main/java/io/trino/gateway/ha/resource/GatewayResource.java Show resolved Hide resolved

amybubu force-pushed the activation-status-logging branch from e155d37 to 47f50c4 Compare

May 11, 2025 23:57

ebyhr reviewed

View reviewed changes

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaGatewayManager.java Show resolved Hide resolved

amybubu added 2 commits

May 12, 2025 20:08


          Improve logging for backend cluster (de)activation

d0d6e73

address nit


          Add error message to activateBackend log

c74ad10

amybubu force-pushed the activation-status-logging branch from 47f50c4 to c74ad10 Compare

May 13, 2025 03:09

amybubu changed the title ~~Modify HaGatewayManager to support informative logging when (de)activating backend clusters~~ Improve logging for backend cluster (de)activation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels