[
https://issues.apache.org/jira/browse/HBASE-19417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16279398#comment-16279398
]
Appy commented on HBASE-19417:
------------------------------
The reason it should be removed is: with current set of two params - map and
hasLoaded- suggests 4 possible combinations. notNull true; notnull false; null
true; null false. But only two can ever happen: null false, notnull true
That's why extra param is not just redundant, but keeping it suggests all the
possibilities which are never there, removing it makes things simpler.
Javadoc of map param should specify significance of null value anyways, and
that'll be enough.
CPs in 2.0 are radically different than 1.x. A lot has changed. It's fine to
change method definitions.
bq. Remove loaded variable in RSRpcServices#bulkLoadHFile
nits. your choice. i suggested it because it'll be used in only one place after
method refactor, and can have directly used map!= null comparison.
> Remove boolean return value from postBulkLoadHFile hook
> -------------------------------------------------------
>
> Key: HBASE-19417
> URL: https://issues.apache.org/jira/browse/HBASE-19417
> Project: HBase
> Issue Type: Bug
> Reporter: Appy
> Assignee: Ted Yu
> Attachments: 19417.v1.txt, 19417.v2.txt, 19417.v3.txt, 19417.v4.txt,
> 19417.v5.txt, 19417.v6.txt, 19417.v7.txt
>
>
> See the discussion at the tail of HBASE-17123 where Appy pointed out that the
> override of loaded should be placed inside else block:
> {code}
> } else {
> // secure bulk load
> map = regionServer.secureBulkLoadManager.secureBulkLoadHFiles(region,
> request);
> }
> BulkLoadHFileResponse.Builder builder =
> BulkLoadHFileResponse.newBuilder();
> if (map != null) {
> loaded = true;
> }
> {code}
> This issue is to address the review comment.
> After several review iterations, here are the changes:
> * Return value of boolean for postBulkLoadHFile() hook are changed to void.
> * Coprocessor hooks (pre and post) are added for the scenario where bulk load
> manager is used.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
