You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "abhishekrb19 (via GitHub)" <gi...@apache.org> on 2026/05/13 03:46:57 UTC
[PR] fix: Make `sys.server_properties` table filterable and resilient to unreachable servers (druid)
abhishekrb19 opened a new pull request, #19459:
URL: https://github.com/apache/druid/pull/19459
Summary:
1. Made `sys.server_properties` filterable, similar to a few other sys tables like `SegmentsTable` — changed from `ScannableTable` to `ProjectableFilterableTable`, enabling Calcite to push down filters (e.g., WHERE server = '...') and projections to the table scan.
2. Graceful error handling — Previously, if any server was unreachable or returned an HTTP error, the entire query threw an exception and failed. Now the table returns a row for that server with property/value as null and the new
error_message column populated with what went wrong.
3. New `error_message` column — this column indicates if properties couldn't be fetched (e.g., "Connection refused", "HTTP 503: Service Unavailable"). Null on success.
Release note:
Added `error_message` column to `sys.server_properties table` and made the table resilient to unreachable servers. Previously, the entire query would fail if any server was unreachable; now a row is returned with `error_message` populated. The table also now supports filter and projection pushdown.
This PR has:
- [x] been self-reviewed.
- [x] added documentation for new or modified features or behaviors.
- [x] a release note entry in the PR description.
- [x] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [x] added integration tests.
- [x] been tested in a test Druid cluster.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] fix: Make `sys.server_properties` table filterable and resilient to unreachable servers (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #19459:
URL: https://github.com/apache/druid/pull/19459#discussion_r3234370473
##########
sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemServerPropertiesTable.java:
##########
@@ -108,42 +121,62 @@ public Schema.TableType getJdbcTableType()
}
@Override
- public Enumerable<Object[]> scan(DataContext root)
+ public Enumerable<Object[]> scan(
+ final DataContext root,
+ final List<RexNode> filters,
Review Comment:
[P2] Actually apply pushed filters before fetching every server
The new ProjectableFilterableTable scan receives Calcite filters, but this method never inspects or removes any accepted filters. Calcite will apply the remaining filters after the scan, so queries such as `WHERE server = 'host:port'` still enumerate all discovered servers and call `/status/properties` for each one. That means an unrelated slow or unreachable node can still delay a query that should have been scoped to one server, and the advertised filter pushdown is not actually implemented. Please parse and consume supported filters, at least `server` and likely `service_name`/`property`, before fetching properties.
##########
sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemServerPropertiesTable.java:
##########
@@ -154,20 +187,34 @@ private Map<String, String> getProperties(DruidNode druidNode)
.get();
if (response.getStatus().getCode() != HttpServletResponse.SC_OK) {
- throw new RE(
- "Failed to get properties from node[%s]. Error code[%d], description[%s].",
- url,
- response.getStatus().getCode(),
- response.getStatus().getReasonPhrase()
- );
+ final String errorMsg = StringUtils.format("HTTP %d: %s",
+ response.getStatus().getCode(),
+ response.getStatus().getReasonPhrase());
+ log.warn("Failed to get properties from node[%s]: error[%s]", url, errorMsg);
+ return new PropertiesResult(new HashMap<>(), errorMsg);
}
- return jsonMapper.readValue(
- response.getContent(),
- new TypeReference<>(){}
+ return new PropertiesResult(
+ jsonMapper.readValue(response.getContent(), new TypeReference<>(){}),
+ null
);
}
catch (Exception e) {
Review Comment:
[P2] Do not turn interrupts into error rows
`Future.get()` can throw `InterruptedException`, but the broad catch now converts it into a synthetic `error_message` row and clears the interrupt status. If the SQL query is cancelled or the execution thread is interrupted during a properties fetch, the scan can keep iterating and return partial table data instead of aborting. Handle `InterruptedException` separately by re-interrupting the thread and propagating cancellation rather than treating it as an unreachable Druid server.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] fix: Make `sys.server_properties` table filterable and resilient to unreachable servers (druid)
Posted by "GWphua (via GitHub)" <gi...@apache.org>.
GWphua commented on code in PR #19459:
URL: https://github.com/apache/druid/pull/19459#discussion_r3232801964
##########
docs/querying/sql-metadata-tables.md:
##########
@@ -322,7 +322,7 @@ SELECT * FROM sys.supervisors WHERE healthy=0;
### SERVER_PROPERTIES table
-The `server_properties` table exposes the runtime properties configured on for each Druid server. Each row represents a single property key-value pair associated with a specific server.
+The `server_properties` table exposes the runtime properties configured on each Druid server. Each row represents a single property key-value pair associated with a specific server. This table supports filter and projection pushdown for efficient querying. If a server is unreachable, the table still returns a row for that server with the `error` column populated instead of failing the entire query.
Review Comment:
```suggestion
The `server_properties` table exposes the runtime properties configured on each Druid server. Each row represents a single property key-value pair associated with a specific server. This table supports filter and projection pushdown for efficient querying. If a server is unreachable, the table still returns a row for that server with the `error_message` column populated instead of failing the entire query.
```
##########
embedded-tests/src/test/java/org/apache/druid/testing/embedded/schema/SystemServerPropertiesTableTest.java:
##########
@@ -146,6 +146,13 @@ public void test_serverPropertiesTable_hiddenProperties()
Assertions.assertFalse(brokerProps.containsKey("password"));
}
+ @Test
+ public void test_serverPropertiesTable_errorMessageColumnExists()
+ {
+ final String result = cluster.runSql("SELECT COUNT(*) FROM sys.server_properties WHERE error_message IS NULL");
+ Assertions.assertFalse(result.isEmpty(), "Should return count of servers with null error_message");
Review Comment:
nit: I think this test just checks if the query did not fail. Can we make the test stronger? For example, asserting a return of a known value (e.g. 0,1,etc.)
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] fix: Make `sys.server_properties` table filterable and resilient to unreachable servers (druid)
Posted by "abhishekrb19 (via GitHub)" <gi...@apache.org>.
abhishekrb19 merged PR #19459:
URL: https://github.com/apache/druid/pull/19459
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] fix: Make `sys.server_properties` table filterable and resilient to unreachable servers (druid)
Posted by "GWphua (via GitHub)" <gi...@apache.org>.
GWphua commented on PR #19459:
URL: https://github.com/apache/druid/pull/19459#issuecomment-4439247148
If this is not urgent, we can wait a day for the AI-assisted review to trigger.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] fix: Make `sys.server_properties` table filterable and resilient to unreachable servers (druid)
Posted by "abhishekrb19 (via GitHub)" <gi...@apache.org>.
abhishekrb19 commented on code in PR #19459:
URL: https://github.com/apache/druid/pull/19459#discussion_r3235373822
##########
sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemServerPropertiesTable.java:
##########
@@ -108,42 +121,62 @@ public Schema.TableType getJdbcTableType()
}
@Override
- public Enumerable<Object[]> scan(DataContext root)
+ public Enumerable<Object[]> scan(
+ final DataContext root,
+ final List<RexNode> filters,
Review Comment:
Yeah, the initial version had some filter pushdown that was slightly complex. Currently, it supports equality filters on the `server` and `service_name` columns. It falls back to querying all servers if it detects any other filter predicate or column.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org