HDDS-8919. Allow EC pipelines to be created and then added to PipelineManager in two steps

[sodonnel]

What changes were proposed in this pull request?

As it stands, the method:

 PipelineManager.createPipeline(ReplicationConfig replicationConfig,
      List<DatanodeDetails> excludedNodes, List<DatanodeDetails> favoredNodes)

Is used to create a new pipeline and add it to the Pipeline manager. This creation and adding is all performed under a single lock The somewhat expensive node selection process from the placement policy is performed under the lock.

For Ratis pipelines, this could be important, as pipelines are created and limited based on the number of existing pipelines on the nodes.

EC pipelines do not have such a limitation, and they also need to be created much more frequently due to their short lived nature.

This PR changes the pipeline manager to have a buildPipeline Method, which calls the placement policy and creates the new pipeline object without any locks.

Then there is an addPipeline method to add the created pipeline to the pipelineManager under the lock.

This will allow for the WritableECPipelineProvider to build pipelines without blocking other threads and then add them to the manager under a lock, reducing the amount of work performed under a lock. This change would need to be made in another PR.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-8919

How was this patch tested?

Modified one test to call the new methods

[sodonnel]

There are two things which concern me about this change:

1. It is fine for EC pipeline to be built and never added to the manager, but for Ratis, it is not. Building a Ratis pipeline actually sends messages to the DNs to establish the Ratis ring. Therefore it might make sense to throw an error in a call comes into BuildPipeline with a Ratis replicationConfig.
2. I was worried about a race condition where we create a pipeline, and then some of the nodes go stale or enter some maintenance state. Both of those events trigger the closing of the pipelines on the affect nodes via the StaleNodeHandler and the StartDatanodeAdminHandler. In both cases, these handlers get the pipeline list on the datanode from NodeManager, which stores it in a concurrent hash map. So even before this change, there is a chance that a pipeline is created, right as the node state changes, and the staleNodeHandler or StartDatanodeAdminHandler will miss the pipeline as it has not made it into the nodeManager map yet. For stale nodes, it would get caught when it goes dead. For decommission / maintenance, I believe it would be a problem (this problem already exists in theory and is no worse due to this change) as the DatanodeAdminMonitorImpl would keep waiting until the pipelines get closed. It would make sense to do something in DatanodeAdminMonitorImpl to trigger another close if it finds pipelines still open.

[guohao-rosicky]

Thanks @sodonnel work on this. This will help the performance of the ec container allocate block.

[adoroszlai]

Thanks @sodonnel for working on this.

Therefore it might make sense to throw an error in a call comes into BuildPipeline with a Ratis replicationConfig.

Agreed. In fact, it may be good to include EC in the method names.

[adoroszlai]

pipeline.getProtobufMessage can now be moved outside the locked section.

[adoroszlai]

Nit: we may want to log the pipeline itself. Previously this was called with a fresh pipeline, but now pipeline can be "anything".

[adoroszlai]

@sodonnel Thanks for updating the PR. It seems there is a test for PipelineManagerImpl failing due to the new replication type check:

Tests run: 22, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 13.469 s <<< FAILURE! - in org.apache.hadoop.hdds.scm.pipeline.TestPipelineManagerImpl
[ERROR] org.apache.hadoop.hdds.scm.pipeline.TestPipelineManagerImpl.testCreatePipeline  Time elapsed: 0.431 s  <<< ERROR!
java.lang.IllegalArgumentException: Replication type must be EC
	at org.apache.hadoop.hdds.scm.pipeline.PipelineManagerImpl.buildECPipeline(PipelineManagerImpl.java:214)
	at org.apache.hadoop.hdds.scm.pipeline.TestPipelineManagerImpl.testCreatePipeline(TestPipelineManagerImpl.java:199)