feat(bulk-load): small update for meta bulk load service #548
Conversation
| void bulk_load_service::initialize_bulk_load_service() | ||
| { | ||
| task_tracker tracker; | ||
| error_code err = ERR_OK; | ||
| _sync_bulk_load_storage.reset(new mss::meta_storage(_meta_svc->get_remote_storage(), &tracker)); |
There was a problem hiding this comment.
Do you intend to set _sync_bulk_load_storage to a temporary tracker?
There was a problem hiding this comment.
Yes, _sync_bulk_load_storage will only be used in functions for sync bulk load info from remote storage, other operations still use _meta_svc->get_meta_storage() and track by meta service tracker.
There was a problem hiding this comment.
OK. Then I suggest making it a local variable, since as you implied, this variable's only used in this function (initialize_bulk_load_service).
There was a problem hiding this comment.
It will be used in functions for sync bulk load info, there are five functions, but the tracker will only be used here, because it should be executed synchronously.
There was a problem hiding this comment.
I remove _sync_bulk_load_storage in this pull request, I will update it in next pull request.
| mss::meta_storage *get_sync_bulk_load_storage() const { return _sync_bulk_load_storage.get(); } | ||
|
|
There was a problem hiding this comment.
return mss::meta_storage & is a little better
There was a problem hiding this comment.
get_sync_bulk_load_storage is a redundant function, I remove it.
This pull request is about some small update for meta bulk load service:
initialize_bulk_load_serviceandon_start_bulk_loadin THREAD_POOL_META_SERVERinitialize_bulk_load_serviceis called bystart_service, it will create bulk load root directory on remote storage and try to get possible bulk load info from it synchronously, operating remote storage data should be executing in THREAD_POOL_META_STATE. if initialize_bulk_load_service executes in it, it will lead dead lock, becausecreate_bulk_load_root_diris a synchronous function.on_start_bulk_loadwill check parameters by reading files on remote file provider(fds), it should not executing in THREAD_POOL_META_STATEpartition_ingestionpartition_ingestionis executing in THREAD_POOL_DEFAULT, while functionhandle_bulk_load_failedandhandle_app_unavailableshould be executed in THREAD_POOL_META_STATEon_control_bulk_loadSpecial explanation:
In pull request #490 , I update simple_kv test case 402, this pull request I recover it into previous state.