[ 
https://issues.apache.org/jira/browse/HDDS-8503?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siddhant Sangwan updated HDDS-8503:
-----------------------------------
    Description: 
The java doc says of chooseNodes says that the nodes it will choose should be 
on racks other than excluded nodes:
{code}
  /**
   * Choose a batch of datanodes on different rack than excludedNodes or
   * chosenNodes.
   * TODO HDDS-7226: Update Implementation to accomodate for already used
   * nodes to conform to existing placement policy.
   *
   * @param excludedNodes - list of the datanodes to excluded. Can be null.
   * @param chosenNodes - list of nodes already chosen. These nodes should also
   *                    be excluded. Cannot be null.
   * @param favoredNodes - list of favoredNodes. It's a hint. Whether the nodes
   *                     are chosen depends on whether they meet the constrains.
   *                     Can be null.
   * @param usedNodes - list of the nodes that are already used.
   * @param favorIndex - the node index of favoredNodes which is not chosen yet.
   * @param nodesRequired - number of datanodes required.
   * @param mapSizeRequired - size required for the container, Ratis metadata.
   * @return List of chosen datanodes.
   * @throws SCMException  SCMException
   */
{code}

But the code looks like it's picking the same rack in one of the if-else 
branches:
{code}
      if (favoredNode != null && networkTopology.isSameParent(
          excludedNodeList.get(excludedNodeList.size() - 1), favoredNode)) {
        chosenNode = favoredNode;
        favorIndex++;
      }
{code}

Should probably be {{!networkTopology.isSameParent(...)}}.

We need to check if this is a bug or not.

  was:
The java doc says of chooseNodes says that the nodes it will choose should be 
on racks other than excluded nodes:
{code}
  /**
   * Choose a batch of datanodes on different rack than excludedNodes or
   * chosenNodes.
   * TODO HDDS-7226: Update Implementation to accomodate for already used
   * nodes to conform to existing placement policy.
   *
   * @param excludedNodes - list of the datanodes to excluded. Can be null.
   * @param chosenNodes - list of nodes already chosen. These nodes should also
   *                    be excluded. Cannot be null.
   * @param favoredNodes - list of favoredNodes. It's a hint. Whether the nodes
   *                     are chosen depends on whether they meet the constrains.
   *                     Can be null.
   * @param usedNodes - list of the nodes that are already used.
   * @param favorIndex - the node index of favoredNodes which is not chosen yet.
   * @param nodesRequired - number of datanodes required.
   * @param mapSizeRequired - size required for the container, Ratis metadata.
   * @return List of chosen datanodes.
   * @throws SCMException  SCMException
   */
{code}

But the code looks like it's picking the same rack in one of the if-else 
branches:
{code}
      if (favoredNode != null && networkTopology.isSameParent(
          excludedNodeList.get(excludedNodeList.size() - 1), favoredNode)) {
        chosenNode = favoredNode;
        favorIndex++;
      }
{code}

Should probably be {{!networkTopology.isSameParent(...)}}.


> Possible bug when excluding rack in SCMContainerPlacementRackAware#chooseNodes
> ------------------------------------------------------------------------------
>
>                 Key: HDDS-8503
>                 URL: https://issues.apache.org/jira/browse/HDDS-8503
>             Project: Apache Ozone
>          Issue Type: Bug
>          Components: SCM
>    Affects Versions: 1.3.0
>            Reporter: Siddhant Sangwan
>            Assignee: Siddhant Sangwan
>            Priority: Major
>
> The java doc says of chooseNodes says that the nodes it will choose should be 
> on racks other than excluded nodes:
> {code}
>   /**
>    * Choose a batch of datanodes on different rack than excludedNodes or
>    * chosenNodes.
>    * TODO HDDS-7226: Update Implementation to accomodate for already used
>    * nodes to conform to existing placement policy.
>    *
>    * @param excludedNodes - list of the datanodes to excluded. Can be null.
>    * @param chosenNodes - list of nodes already chosen. These nodes should 
> also
>    *                    be excluded. Cannot be null.
>    * @param favoredNodes - list of favoredNodes. It's a hint. Whether the 
> nodes
>    *                     are chosen depends on whether they meet the 
> constrains.
>    *                     Can be null.
>    * @param usedNodes - list of the nodes that are already used.
>    * @param favorIndex - the node index of favoredNodes which is not chosen 
> yet.
>    * @param nodesRequired - number of datanodes required.
>    * @param mapSizeRequired - size required for the container, Ratis metadata.
>    * @return List of chosen datanodes.
>    * @throws SCMException  SCMException
>    */
> {code}
> But the code looks like it's picking the same rack in one of the if-else 
> branches:
> {code}
>       if (favoredNode != null && networkTopology.isSameParent(
>           excludedNodeList.get(excludedNodeList.size() - 1), favoredNode)) {
>         chosenNode = favoredNode;
>         favorIndex++;
>       }
> {code}
> Should probably be {{!networkTopology.isSameParent(...)}}.
> We need to check if this is a bug or not.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to