[jira] [Commented] (GIRAPH-105) BspServiceMaster.checkWorkers() should return empty lists instead of null

["Avery Ching (Commented) (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/GIRAPH-105?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13171793#comment-13171793 ] 

Avery Ching commented on GIRAPH-105:
------------------------------------

I totally agree that the NPE is messy for a situation that can occur if the resources aren't met.  However, returning a valid list could be a little confusing since this method must return the minimum at the very least.  What about leaving the null returns for checkWorkers, but change the javadoc of checkWorkers() from 

{noformat}
    /**
     * Check all the {@link WorkerInfo} objects to ensure that a minimum
     * number of good workers exists out of the total that have reported.
     *
     * @return List of of healthy workers
     */
{noformat}

to something like:

{noformat}
    /**
     * Check all the {@link WorkerInfo} objects to ensure that a minimum
     * number of good workers exists out of the total that have reported.
     *
     * @return List of healthy workers such that the minimum has been 
     *         met, otherwise null
     */
{noformat}

and then changing 

{noformat}
        if (healthyWorkerInfoList.isEmpty()) {
{noformat}

to 

{noformat}
        if (healthyWorkerInfoList == null) {
{noformat}

Thoughts?
                
> BspServiceMaster.checkWorkers() should return empty lists instead of null
> -------------------------------------------------------------------------
>
>                 Key: GIRAPH-105
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-105
>             Project: Giraph
>          Issue Type: Bug
>    Affects Versions: 0.70.0
>            Reporter: Sebastian Schelter
>            Priority: Minor
>         Attachments: GIRAPH-105.patch
>
>
> BspServiceMaster.checkWorkers() is invoked in BspServiceMaster.coordinateSuperstep() and in BspServiceMaster.createInputSplits(). Both check for an empty list to fail the job in case something has gone wrong. However, checkWorkers() returns null in case of problems, causing an NPE in the calling code.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira