Conversation
…ing and pauseless flow
1. Segment commit won't create a new segment for the pauseless path as the segment has already been created at the commit start 2. Segment commit will create a new segment if the conventional path is chosen
This reverts commit 717895b.
… is a controller failure. The server would just now persist the segment to disk instead of retrying.
… just download url
9aman
force-pushed
the
feature_pauseless
branch
from
ae1889c to
e5ba623
Compare
| LOGGER.info("Starting realtime segment manager"); | ||
| _pinotLLCRealtimeSegmentManager = | ||
| new PinotLLCRealtimeSegmentManager(_helixResourceManager, _config, _controllerMetrics); | ||
| _pinotLLCRealtimeSegmentManager = createPinotLLCRealtimeSegmentManager(); |
There was a problem hiding this comment.
These changes in the ControllerStarter have been added to easily override two crucial classes:
PinotLLCRealtimeSegmentManager and RealtimeSegmentValidationManager.
| // TODO (akkhanch): introducing this as the first step of the segment metadata might have succeeded. We don't | ||
| // want the server to try indefinitely, rather we would rely on the validation manager to complete the remaining | ||
| // steps. | ||
| if (segmentMetadata.getStatus() == CommonConstants.Segment.Realtime.Status.DONE |
There was a problem hiding this comment.
This has been added to prevent servers from retrying indefinitely.
| case COMMITTER_NOTIFIED: | ||
| return committerNotifiedExtendBuildTime(instanceId, offset, extTimeSec, now); | ||
| if (!_pauselessConsumptionEnabled) { | ||
| return committerNotifiedExtendBuildTime(instanceId, offset, extTimeSec, now); |
There was a problem hiding this comment.
Change in the commit protocol changes the state in which we get the extendBuildTime request
| if (offset.compareTo(_winningOffset) == 0) { | ||
| // Wait until winner has posted the segment before asking this server to KEEP the segment. | ||
| response = hold(instanceId, offset); | ||
| // Keep if it's pauseless enabled |
There was a problem hiding this comment.
We allow the replicas to build the segment. This ensures that:
Reduced need for disaster recovery: The server is persisted to disk on atleast one of the servers.Continued ingestion on replicas: The replicas can continue ingesting newly created segments rather than waiting for the committing server to complete the commit (existing behavior).
|
|
||
| // TODO: might need to support the two ways in which stream config can be set. On under the ingestion | ||
| // config and other under the tableIndexConfig | ||
| if (isPauselessEnabeld()) { |
There was a problem hiding this comment.
We want to Initiate the COMMIT_START before building the segment. This is crucial for allowing ingestion while the current segment is being built.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #14460 +/- ##
============================================
- Coverage 61.75% 55.57% -6.18%
- Complexity 207 796 +589
============================================
Files 2436 2099 -337
Lines 133233 110518 -22715
Branches 20636 17528 -3108
============================================
- Hits 82274 61420 -20854
+ Misses 44911 44234 -677
+ Partials 6048 4864 -1184
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| private long _memoryUsedBytes; | ||
| private long _segmentSizeBytes; | ||
| private String _streamPartitionMsgOffset; | ||
| private boolean _pauselessConsumptionEnabled; |
There was a problem hiding this comment.
Instead of changing the existing protocol, please add a new controller endpoint that is called from the server when it needs the idealstate modified before segment upload.
This will avoid a lot of if pauselessEnabled statements in the code path
|
Accepted #14741 instead |
4 participants
[DRAFT]
The PR covers changes for to allow consumption during segment build and upload.
Following are the core changes:
Commit Protocol Changes:
Additional parameter to signify that the pauseless is enabled.
- Need: This has been added to prevent issues that might arise due to change in table config during Commit protocol.
- Alternative: Fetch the value from tableConfig instead.
Server Side Changes
RealtimeSegmentDataManagerRealtimeTableDataManagerController Side Changes
SegmentCompletionManagerPinotLLCRealtimeSegmentManager