-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Preload segments #38
base: master_upstream
Are you sure you want to change the base?
Preload segments #38
Conversation
f6ee594
to
530675f
Compare
// But if table mgr is not created or the segment is not loaded yet, the localMetadata | ||
// is set to null. Then, addOrReplaceSegment method will load the segment accordingly. | ||
SegmentMetadata localMetadata = getSegmentMetadata(tableDataManager, tableNameWithType, segmentName); | ||
tableDataManager.addOrReplaceSegment(segmentName, new IndexLoadingConfig(tableDataManager.getTableDataManagerConfig().getInstanceDataManagerConfig(), tableConfig, schema), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the changes of batch loading segments look good to me.
and fyi, the methods to sync between loading and ready phases
- the setReady() is supposed to be called at the end of preloadSegments()
- I'm planning to call untilReady() in most segment mgmt methods on RealtimeTableDataManager, e.g. addSegment(), remoteSegment(), and also this addOrReplaceSegment(). So directly calling it would lead to deadlock. I'll think about either add a boolean on addOrReplaceSegment() or add a new method, w/o waiting on the condition.
// a feature flag to enable preloading; or simply _isReady = true, thus no blocking to handle helix state transitions
@Override
public void untilReady()
throws InterruptedException {
if (_isReady) { // volatile
return;
}
_isReadyLock.lock();
try {
while (!_isReady) {
_isReadyCon.await();
}
} finally {
_isReadyLock.unlock();
}
}
private void setIsReady() {
_isReadyLock.lock();
try {
_isReady = true;
_isReadyCon.signalAll();
} finally {
_isReadyLock.unlock();
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the RealtimeTableDataManager.addSegment() should be called here to add segment. The addOrReplaceSegment() seems for the Offline table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, addOrReplaceSegment
is the only method that's called by HelixInstanceDataManager iiuc.
Internally, it calls the addSegment
and passed the required params such as segment metadata
…or better performance
This won't compile right now. Mostly meant to track progress.