[Improve] StarRocksSourceReader use the existing client #6480
Conversation
| @Override | ||
| public void open() throws Exception {} | ||
| public void open() throws Exception { | ||
| clientsPools = new LinkedHashMap<>(5); |
There was a problem hiding this comment.
Just HashMap enough.
| clientsPools = new LinkedHashMap<>(5); | |
| clientsPools = new HashMap<>(); |
| if (!clientsPools.isEmpty()) { | ||
| clientsPools.values().forEach(StarRocksBeReadClient::close); | ||
| } |
There was a problem hiding this comment.
Please add try catch in forEach. We should make sure all client will be invoked close method.
There was a problem hiding this comment.
Please add try catch in
forEach. We should make sure all client will be invoked close method.
done, please recheck at your convenience.
|
cc @531651225 |
| throw new StarRocksConnectorException( | ||
| CLOSE_BE_READER_FAILED, e); |
There was a problem hiding this comment.
Do not throw exception. It will break logic of close other client. Just log.error.
There was a problem hiding this comment.
done, please recheck at your convenience.
|
Please follow the guide to open ci on your fork repository. Thanks. |
Thank you very much, I will try to complete it according to your guidance |
3 participants
Purpose of this pull request
StarRocksSourceReader reads partition data using the existing client instead of re-establishing a connection for each partition.
Does this PR introduce any user-facing change?
no
How was this patch tested?
exist tests.
Check list
New License Guide
release-note.