HDDS-6358. EC: Refactor ECKeyOutputStream#write()

[kaijchen]

What changes were proposed in this pull request?

Refactor the code to make it cleaner, here is a summary of the changes.

1. Instead of treating head/middle/tail differently, use a general way to write data into buffers.
2. No more allocateBlockIfNeeded parameters being passed around.
3. Stream will be closed if any exception happens, and IOException will be thrown.

What is the link to the Apache JIRA

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

How was this patch tested?

Unit tests.

[guihecheng]

Hi @kaijchen the overall patch LGTM!
It's better to have several lines of summary about the changes made here to be easier for reviewing, I believe that there are certain points to be mentioned that is more than cleanup.
cc @umamaheswararao

[kaijchen]

@guihecheng Thanks for reviewing, I have edited the PR message to include a summary of changes.

[umamaheswararao]

Thanks for filing this @kaijchen
Initially journey was started with this single loop
However for easy trace tracking at which position write impacted, I have decided to split into 3 sections as you got that in summary comments.
I think now, we have evolved enough and bit stable compared to starting point where we need lot of debug help info to track down issues. ( When it has 3 sections I found helpful as the trace indicates which position of data sizes it's causing issues. We may need to add more debug info as well to improve debug-ability.)

I will review it soon and provide my feedback if any.

Thanks a lot for working on this.

[umamaheswararao]

@kaijchen, Thanks for working on this task. I have dropped few comments. Please check.

[umamaheswararao]

I am wondering, whether this allocateBlockINeeded() can be under currentStreamEntry().getRemaining()>=0 condition ? That can avoid at least couple of if checks every time.

[kaijchen]

Yes. But will there be noticeable performance difference? Because KeyOutputStream is doing the same thing.
If we need to add this check, can we add it inside allocateBlockIfNeeded()?

[umamaheswararao]

Few if checks might not contribute to significant improvement, but this is key path and saving such thing also helps. But doing that checks needs to modify BlockOutputStreamEntryPool, that should be done in master I feel as that is common. So, I am ok to keep it here now as it is.

[kaijchen]

Hi @umamaheswararao, thanks for the explanation and the review.
I have updated the patch to resolve most of the comments. And there is one needs further discussion.
I think we can let block allocation happen here instead of inside handleDataWrite or handleParityWrite.
What do you think?

[umamaheswararao]

LGTM, pending CI

[umamaheswararao]

Few if checks might not contribute to significant improvement, but this is key path and saving such thing also helps. But doing that checks needs to modify BlockOutputStreamEntryPool, that should be done in master I feel as that is common. So, I am ok to keep it here now as it is.