Track unavailable segments in InstanceSelector #5337
Conversation
|
@fx19880617 This PR changed the broker debug API to return the unavailable segments within the routing table. Please check whether it is okay for presto connector |
Jackie-Jiang
force-pushed
the
routing_unavailable_segments
branch
2 times, most recently
from
e5fa585
to
584eea6
Compare
Codecov Report
@@ Coverage Diff @@
## master #5337 +/- ##
==========================================
+ Coverage 56.88% 66.29% +9.41%
==========================================
Files 1073 1075 +2
Lines 54722 54775 +53
Branches 8159 8169 +10
==========================================
+ Hits 31126 36313 +5187
+ Misses 21149 15786 -5363
- Partials 2447 2676 +229
Continue to review full report at Codecov.
|
| Map<String, List<String>> segmentToEnabledInstancesMap = _segmentToEnabledInstancesMap; | ||
| Set<String> currentUnavailableSegments = _unavailableSegments; |
There was a problem hiding this comment.
we dont need this variable, we can use _unavailableSegments in line 110 right?
There was a problem hiding this comment.
It should be fine, it's just a reference.
There was a problem hiding this comment.
_unavailableSegments is volatile, so I find get its reference for performance concern
| } | ||
|
|
||
| @Test | ||
| public void testUnavailableSegments() { |
There was a problem hiding this comment.
Good to iterate this test a few times, with realtime table and with CONSUMING segments as well
| Map<String, List<String>> segmentToEnabledInstancesMap = _segmentToEnabledInstancesMap; | ||
| Set<String> currentUnavailableSegments = _unavailableSegments; |
There was a problem hiding this comment.
It should be fine, it's just a reference.
| } | ||
| } | ||
| _unavailableSegments = newUnavailableSegments; | ||
| } | ||
|
|
||
| @Override | ||
| public void onExternalViewChange(ExternalView externalView, Set<String> onlineSegments) { |
There was a problem hiding this comment.
It might be good to put some comments on what you're gonna do in this method, as the logic becomes complex.
Discussed offline. Will keep the API unchanged. |
Unavailable segments are the segments that has no enabled instance or all enabled instances are in ERROR state. We don't count segment with enabled instance in OFFLINE state as unavailable because it is a valid state when segment is new added and has not become ONLINE/CONSUMING. Introduced RoutingTable class to wrap the a map from ServerInstance to segments and a list of unavailable segments. Added a new property 'numUnavailableSegments' into the RequestStatistics.
Jackie-Jiang
force-pushed
the
routing_unavailable_segments
branch
from
584eea6
to
236ce24
Compare
Unavailable segments are the segments that has no enabled instance or all enabled instances are in ERROR state.
We don't count segment with enabled instance in OFFLINE state as unavailable because it is a valid state when segment is new added and has not become ONLINE/CONSUMING.
Introduced RoutingTable class to wrap the a map from ServerInstance to segments and a list of unavailable segments.
Added a new property 'numUnavailableSegments' into the RequestStatistics.