Skip to content

HDDS-5174. EC: Allow EC blocks to be requested from OM#2414

Merged
umamaheswararao merged 2 commits intoapache:HDDS-3816-ecfrom
sodonnel:ec-alloc-block-HDDS-5174
Jul 26, 2021
Merged

HDDS-5174. EC: Allow EC blocks to be requested from OM#2414
umamaheswararao merged 2 commits intoapache:HDDS-3816-ecfrom
sodonnel:ec-alloc-block-HDDS-5174

Conversation

@sodonnel
Copy link
Contributor

What changes were proposed in this pull request?

Allow EC block requests to propagate from OM to SCM, as currently only STANDALONE and RATIS are supported.

What is the link to the Apache JIRA

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

How was this patch tested?

Existing tests.

@sodonnel sodonnel changed the title HDDS-5174. EC: Allow EC blocks to be requests from OM HDDS-5174. EC: Allow EC blocks to be requested from OM Jul 14, 2021
@sodonnel sodonnel requested review from elek and umamaheswararao July 14, 2021 22:23
Copy link
Contributor

@umamaheswararao umamaheswararao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes looks straightforward to me.
We also discussed, testing allocate block without making server calls is not easy. we can be tested as part of full write path test.

@umamaheswararao umamaheswararao merged commit cd781b9 into apache:HDDS-3816-ec Jul 26, 2021
break;
case EC:
requestBuilder.setEcReplicationConfig(
((ECReplicationConfig)replicationConfig).toProto());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we missed break here. Sorry missed this in previous review.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is merged, Let's handle in next patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Well spotted. I will create a small patch today to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants