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

Ivan Andika updated HDDS-15729:
-------------------------------
    Description: 
The PlacementPolicy (both Pipeline Placement Policy and the Container Placement 
Policy) is poorly abstracted which is very hard to reason about. There are a 
lot of leaky abstractions or logic specific to a replication config (e.g. Ratis 
logic in interface that can also be applied to EC). I found myself getting lost 
every time I look at the placement policy logic since it's hard to build a 
mental model of the whole process. This task is to identify these abstractions 
and fix them for better readability and maintainability, as well as document 
the exact contracts and responsibilities of these policies. This will minimize 
any surprises and potential bugs due to misunderstandings.

For example (not exhaustive)
 * PlacementPolicy is supposed to be generic, but we have container placement 
specific logic like validateContainerPlacement and 
replicasToCopyToFixMisReplication and replicasToRemoveToFixOverreplication
 * PipelinePlacementPolicy seems to only used for RATIS pipeline, but we have 
EC pipeline, so it gives an impression that PipelinePlacementPolicy can also be 
used for EC, which AFAIK is not the case.

 * 
 ** PipelinePlacementPolicy is only used for RATIS, then we should make it 
explicit by changing the name to RatisPipelinePlacementPolicy
 ** PipelinePlacementPolicy also have internal "Ratis" related pipelines like 
 * We are still using PlacementPolicy for everything, even if 
PipelinePlacementPolicy is more appropriate for pipeline related matters, which 
forces PlacementPolicy to implement interfaces that it should not be 
implementing
 ** For example, in RatisPipelineProvider we can change PlacementPolicy to 
PipelinePlacementPolicy since we are dealing with pipeline
 * Domain word like "anchor" node is not properly defined

  was:
The PlacementPolicy (both Pipeline Placement Policy and the Container Placement 
Policy) is poorly abstracted which is very hard to reason about. There are a 
lot of leaky abstractions or logic specific to a replication config (e.g. Ratis 
logic in interface that can also be applied to EC). I found myself getting lost 
every time I look at the placement policy logic since it's hard to build a 
mental model of the whole process. This task is to identify these abstractions 
and fix them for better readability and maintainability, as well as document 
the exact contracts and responsibilities of these policies. This will minimize 
any surprises and potential bugs due to misunderstandings.

For example (not exhaustive)
 * PlacementPolicy is supposed to be generic, but we have container placement 
specific logic like validateContainerPlacement and 
replicasToCopyToFixMisReplication and replicasToRemoveToFixOverreplication
 * PipelinePlacementPolicy seems to only used for RATIS pipeline, but we have 
EC pipeline, so it gives an impression that PipelinePlacementPolicy can also be 
used for EC, which AFAIK is not the case.

 * 
 ** PipelinePlacementPolicy is only used for RATIS, then we should make it 
explicit by changing the name to RatisPipelinePlacementPolicy
 ** PipelinePlacementPolicy also have internal "Ratis" related pipelines like 
 * We are still using PlacementPolicy for everything, even if 
PipelinePlacementPolicy is more appropriate for pipeline related matters, which 
forces PlacementPolicy to implement interfaces that it should not be 
implementing
 ** For example, in RatisPipelineProvider we can change PlacementPolicy to 
PipelinePlacementPolicy since we are dealing with pipeline


> Improve the PlacementPolicy code structure
> ------------------------------------------
>
>                 Key: HDDS-15729
>                 URL: https://issues.apache.org/jira/browse/HDDS-15729
>             Project: Apache Ozone
>          Issue Type: Improvement
>            Reporter: Ivan Andika
>            Assignee: Ivan Andika
>            Priority: Major
>
> The PlacementPolicy (both Pipeline Placement Policy and the Container 
> Placement Policy) is poorly abstracted which is very hard to reason about. 
> There are a lot of leaky abstractions or logic specific to a replication 
> config (e.g. Ratis logic in interface that can also be applied to EC). I 
> found myself getting lost every time I look at the placement policy logic 
> since it's hard to build a mental model of the whole process. This task is to 
> identify these abstractions and fix them for better readability and 
> maintainability, as well as document the exact contracts and responsibilities 
> of these policies. This will minimize any surprises and potential bugs due to 
> misunderstandings.
> For example (not exhaustive)
>  * PlacementPolicy is supposed to be generic, but we have container placement 
> specific logic like validateContainerPlacement and 
> replicasToCopyToFixMisReplication and replicasToRemoveToFixOverreplication
>  * PipelinePlacementPolicy seems to only used for RATIS pipeline, but we have 
> EC pipeline, so it gives an impression that PipelinePlacementPolicy can also 
> be used for EC, which AFAIK is not the case.
>  * 
>  ** PipelinePlacementPolicy is only used for RATIS, then we should make it 
> explicit by changing the name to RatisPipelinePlacementPolicy
>  ** PipelinePlacementPolicy also have internal "Ratis" related pipelines like 
>  * We are still using PlacementPolicy for everything, even if 
> PipelinePlacementPolicy is more appropriate for pipeline related matters, 
> which forces PlacementPolicy to implement interfaces that it should not be 
> implementing
>  ** For example, in RatisPipelineProvider we can change PlacementPolicy to 
> PipelinePlacementPolicy since we are dealing with pipeline
>  * Domain word like "anchor" node is not properly defined



--
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