Skip to content

Conversation

zyy17
Copy link
Collaborator

@zyy17 zyy17 commented Oct 2, 2025

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

What's changed and what's your intention?

Related greptime-proto PR: GreptimeTeam/greptime-proto#278.

Add peer_hostname field in information_schema.cluster_info table.

PR Checklist

Please convert it to a draft if some of the following conditions are not met.

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR requires documentation updates.
  • API changes are backward compatible.
  • Schema or data changes are backward compatible.

@github-actions github-actions bot added size/S docs-not-required This change does not impact docs. labels Oct 2, 2025
@zyy17 zyy17 force-pushed the refactor/add-hostname-field branch 2 times, most recently from 4da371e to 354ebd4 Compare October 2, 2025 16:10
@zyy17 zyy17 marked this pull request as ready for review October 3, 2025 05:36
@zyy17 zyy17 requested review from a team, MichaelScofield, waynexia and zhongzc as code owners October 3, 2025 05:36
@zyy17 zyy17 requested review from WenyXu and fengjiachun and removed request for waynexia and zhongzc October 3, 2025 05:37
@zyy17 zyy17 enabled auto-merge October 3, 2025 05:39
@killme2008
Copy link
Contributor

What’s the purpose of this field? Isn’t peer_addr sufficient? @zyy17

FYI: I recommend naming the table field peer_hostname (hostname is fine in the proto).

@zyy17
Copy link
Collaborator Author

zyy17 commented Oct 15, 2025

What’s the purpose of this field? Isn’t peer_addr sufficient? @zyy17

FYI: I recommend naming the table field peer_hostname (hostname is fine in the proto).

Most of the time, peer_addr will be in IP format with a port. It isn't easy to read and use.

hostname is also the key information and is human-readable. Also, the 3rd tool can use this info to fetch the monitoring data.

@zyy17 zyy17 force-pushed the refactor/add-hostname-field branch from ccc0549 to bcef0cd Compare October 15, 2025 07:26
@zyy17 zyy17 requested a review from v0y4g3r as a code owner October 15, 2025 07:26
@zyy17 zyy17 changed the title refactor: add hostname field in information_schema.cluster_info table refactor: add peer_hostname field in information_schema.cluster_info table Oct 15, 2025
@zyy17 zyy17 force-pushed the refactor/add-hostname-field branch 2 times, most recently from d996598 to 0b33b63 Compare October 15, 2025 09:23
@zyy17 zyy17 force-pushed the refactor/add-hostname-field branch from 0b33b63 to 5b31c17 Compare October 15, 2025 09:28
@zyy17 zyy17 requested a review from WenyXu October 15, 2025 09:35
@zyy17
Copy link
Collaborator Author

zyy17 commented Oct 15, 2025

@sunng87, I added some todo!() for the new JSON type to adapt the new proto, PTAL.

@zyy17 zyy17 requested a review from sunng87 October 15, 2025 14:46
@zyy17 zyy17 removed request for a team and v0y4g3r October 15, 2025 14:47
@zyy17 zyy17 disabled auto-merge October 15, 2025 14:55
@zyy17 zyy17 force-pushed the refactor/add-hostname-field branch from f8b3552 to aba1b01 Compare October 16, 2025 03:16
@zyy17 zyy17 force-pushed the refactor/add-hostname-field branch from aba1b01 to 8c4e0b1 Compare October 16, 2025 03:17
Copy link
Member

@WenyXu WenyXu left a comment

Choose a reason for hiding this comment

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

LGTM

@zyy17 zyy17 force-pushed the refactor/add-hostname-field branch from ba71194 to 766447d Compare October 16, 2025 03:43
@zyy17 zyy17 enabled auto-merge October 16, 2025 04:05
@zyy17 zyy17 added this pull request to the merge queue Oct 16, 2025
Merged via the queue into GreptimeTeam:main with commit 6ab4672 Oct 16, 2025
42 checks passed
@zyy17 zyy17 deleted the refactor/add-hostname-field branch October 16, 2025 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-not-required This change does not impact docs. size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants