kfaraz commented on code in PR #18547:
URL: https://github.com/apache/druid/pull/18547#discussion_r2371616037
##########
server/src/main/java/org/apache/druid/server/DruidNode.java:
##########
@@ -127,7 +130,8 @@ public DruidNode(
@JacksonInject @Named("servicePort") @JsonProperty("port") Integer port,
@JacksonInject @Named("tlsServicePort") @JsonProperty("tlsPort") Integer
tlsPort,
@JsonProperty("enablePlaintextPort") Boolean enablePlaintextPort,
- @JsonProperty("enableTlsPort") boolean enableTlsPort
+ @JsonProperty("enableTlsPort") boolean enableTlsPort,
+ @JsonProperty("labels") Map<String, String> labels
Review Comment:
```suggestion
@JsonProperty("labels") @Nullable Map<String, String> labels
```
##########
docs/querying/sql-metadata-tables.md:
##########
@@ -236,6 +236,7 @@ Servers table lists all discovered servers in the cluster.
|max_size|BIGINT|Max size in bytes this server recommends to assign to
segments see
[druid.server.maxSize](../configuration/index.md#historical-general-configuration).
Only valid for HISTORICAL type, for other types it's 0|
|is_leader|BIGINT|1 if the server is currently the 'leader' (for services
which have the concept of leadership), otherwise 0 if the server is not the
leader, or null if the server type does not have the concept of leadership|
|start_time|STRING|Timestamp in ISO8601 format when the server was announced
in the cluster|
+|labels|VARCHAR|Labels for the server see
[druid.labels](../configuration/index.md)|
Review Comment:
```suggestion
|labels|VARCHAR|Labels for the server configured using the property
[`druid.labels`](../configuration/index.md)|
```
##########
server/src/main/java/org/apache/druid/server/DruidNode.java:
##########
@@ -89,6 +89,9 @@ public class DruidNode
"unknown"
);
+ @JsonProperty
+ private Map<String, String> labels;
Review Comment:
```suggestion
private final Map<String, String> labels;
```
##########
sql/src/test/java/org/apache/druid/sql/calcite/schema/SystemSchemaTest.java:
##########
@@ -1077,10 +1092,11 @@ private Object[] createExpectedRow(
@Nullable Long currSize,
@Nullable Long maxSize,
@Nullable Long isLeader,
- String startTime
+ String startTime,
+ Map<String, String> labels
)
{
- return new Object[]{
+ return new Object[] {
Review Comment:
Nit: not needed
##########
website/.spelling:
##########
@@ -2070,6 +2070,7 @@ druid.enableTlsPort
druid.indexer.autoscale.workerVersion
druid.service
druid.storage.disableAcl
+druid.labels
Review Comment:
This shouldn't be needed if we quote the property name as `druid.labels` in
the docs.
##########
server/src/test/java/org/apache/druid/server/DruidNodeTest.java:
##########
Review Comment:
Shouldn't we update a test to verify non-null values of `labels`?
##########
server/src/main/java/org/apache/druid/server/DruidNode.java:
##########
@@ -208,6 +214,12 @@ private void init(
this.serviceName = serviceName;
this.host = host;
this.bindOnHost = bindOnHost;
+ this.labels = labels;
+ }
+
+ public Map<String, String> getLabels()
Review Comment:
```suggestion
@Nullable
public Map<String, String> getLabels()
```
##########
sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java:
##########
@@ -180,6 +180,7 @@ public class SystemSchema extends AbstractSchema
.add("max_size", ColumnType.LONG)
.add("is_leader", ColumnType.LONG)
.add("start_time", ColumnType.STRING)
+ .add("labels", ColumnType.NESTED_DATA)
Review Comment:
I don't think any of the other columns use nested data as the type.
Even columns in `sys.segments` like `last_compaction_state`, `dimensions`,
`metrics`, etc
use `STRING` as the type. I think we should just stick to that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
