HDDS-4942. EC: Implement ECBlockInputStream to read a single EC BlockGroup

[sodonnel]

What changes were proposed in this pull request?

Implement the happy-path read, which does not support "degraded read" (on the fly EC recovery), to read the data from a single EC block group.

What is the link to the Apache JIRA

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

How was this patch tested?

New unit tests

[umamaheswararao]

Overall approach looks good to me. I have added few minor comments. Please check when possible. Thanks

[umamaheswararao]

We can use factory when we need to create different category of objects of same type. Do you have plan to create different type of blockInputStream here? If yes, then what parameter decides the type?

[fapifta]

I agree that this is more or less a strategy instead of a factory, in the sense of how it is used, on the other hand it is a strategy, given to the ECBlockInputstream, which provides regular BlockInputStream instances. In this sense it is creating objects. I checked into if it can be implemented as a functional interface, and passed in as a lambda, but Stephen is right, that would not make the code any more simpler or readable.
If we agree (and I am) that factory would be an overload of the factory design concept, with different meaning here, then a good alternative might be BlockInputStreamProvider, or BlockInputStreamCreator as the name, but the code itself is fine. (Though, if we go with provider, I would change the method name to provide instead of create.)

One more thing that I would question here, is the separation of the refreshFunction and the xceiverClientFactory into an internal state inside this type. I am unsure, but most likely ECBlockInputStream will be used from KeyInputStream#addStream(), and in that case, we can pass on both the client factory and the refresh function to the create/provide method there, and nulls in the current tests.

[sodonnel]

One more thing that I would question here, is the separation of the refreshFunction and the xceiverClientFactory into an internal state inside this type. I am unsure, but most likely ECBlockInputStream will be used from KeyInputStream#addStream(), and in that case, we can pass on both the client factory and the refresh function to the create/provide method there, and nulls in the current tests.

I think the reason I put them into the type was to cut down the number of parameters getting passed from ECKeyInputStream (which does not exist yet) to ECBlockInputStream. One instance of the factory / provider can be created in ECKeyInputStream and then passed to the ECBlockInputStream constructor. It does not care about or use the xceiverFactory or refreshFunction except to pass them onwards to the underlying BlockInputStream . Right now, I had that inside the factory / provider, rather than storing them as instance variables in ECBlockInputStream.

To make this change, I would need to add two parameters variables to the ECBlockInputStream constructor and then store them as instance variables within the class, and then pass them to the create() method.

If you feel it makes things better, I am happy to make the change I suggested above.

then a good alternative might be BlockInputStreamProvider,

OK - I will change the name to provider and the method to "provide" rather than create.

[umamaheswararao]

+1 Latest changes LGTM. ( except a small nit about comment.)
Pending nit fix and green CI.

[umamaheswararao]

Nit: Please change the comment with the right class name.