broker & server api for realtime table freshness #13249
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #13249 +/- ##
============================================
+ Coverage 61.75% 62.12% +0.37%
+ Complexity 207 198 -9
============================================
Files 2436 2534 +98
Lines 133233 139101 +5868
Branches 20636 21549 +913
============================================
+ Hits 82274 86417 +4143
- Misses 44911 46227 +1316
- Partials 6048 6457 +409
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
From high level, I'd suggest adding the API to the controller side and follow these steps:
- Look at IS of the table and find all consuming segments
- Ask servers hosting the consuming segments about the freshness of the target table
- Collect result and respond
Having partition level freshness could also be useful for debugging purpose
| @ApiResponse(code = 404, message = "Table not found"), | ||
| @ApiResponse(code = 500, message = "Internal server error") | ||
| }) | ||
| public Map<String, Long> getSegmentsFreshnessInfo( |
There was a problem hiding this comment.
I think we want to get the table level freshness value (minimum of freshness value across all streaming partitions) and return it as one single long value. Do you see the need of return per segment freshness? Ideally it should be directly pulled from RealtimeTableDataManager
There was a problem hiding this comment.
you're right that the minimum freshness computation can be done here to avoid having to push the map of the segments as part of the response
this does go against the idea of having partition level freshness info for debugging purposes. What do you suggest?
There was a problem hiding this comment.
I guess we can provide 2 APIs (or combining 2 functionalities into one API), one returning the per partition info, one return the table level info. The API should only take the table name as input
|
@Jackie-Jiang regarding broker vs controller - I was relying on the broker being aware of which segments can be queried or not. (ie, we know some servers hosting segments are not ready as they are not yet caught up). Additionally, the broker routing map means we can cycle through the replica's based on the query strategy of the table to determine the freshness we have some FUD about adding to controller for above reasons & controller is not generally a high QPS or reliable component but broker is expected to be |
| @ApiParam(value = "Name of the table", example = "myTable | myTable_REALTIME", | ||
| required = true) @PathParam("tableName") String tableName, | ||
| @ApiParam(value = "Timeout in milliseconds") @QueryParam("timeoutMs") @DefaultValue("10000") long timeoutMs) { | ||
| String realtimeTableName = TableNameBuilder.REALTIME.tableNameWithType(tableName); |
There was a problem hiding this comment.
If you call this API with an offline table, this line will just append _REALTIME to the table name. e.g. XXX_OFFLINE -> XXX_REALTIME
Then it will pass the offline check on line 134
I think we should check if it's an offline table first
One of the potential solutions to #12477 was a broker api that returns table freshness info.
This PR:
/debug/tableFreshness/{table}?timeoutMs=1000that returns the minimum ingestion lag timestamp reported by servers across all the consuming segments. Underneath, the broker uses a new server api,/tables/{table}/consumingSegmentsFreshnessInfothat takes in a list of segments and returns the ingestion lag timestamp for the consuming segments among themExample usage from broker:
Example server usage:
Possible improvements in future PRs:
cc @Jackie-Jiang
Instructions:
featurebugfixperformanceuibackward-incompatrelease-notes(**)(*) Other labels to consider:
testingdependenciesdockerkubernetesobservabilitysecuritycode-styleextension-pointrefactorcleanup(**) Use
release-noteslabel for scenarios like: