Skip to content

HDDS-5741. EC: Remove hard coded chunksize and get from from ReplicationConfig#2687

Merged
sodonnel merged 3 commits intoapache:HDDS-3816-ecfrom
sodonnel:ec-HDDS-5741-hardcoded-chunksize
Oct 4, 2021
Merged

HDDS-5741. EC: Remove hard coded chunksize and get from from ReplicationConfig#2687
sodonnel merged 3 commits intoapache:HDDS-3816-ecfrom
sodonnel:ec-HDDS-5741-hardcoded-chunksize

Conversation

@sodonnel
Copy link
Copy Markdown
Contributor

@sodonnel sodonnel commented Sep 28, 2021

What changes were proposed in this pull request?

The EC Chunksize is currently hard coded in ECBlockInputStream and ECBlockOutputStream. Now that ECReplicationConfig carries the chunkSize and codec, we can remove these hardcoded values.

What is the link to the Apache JIRA

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

How was this patch tested?

Existing tests

Copy link
Copy Markdown
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.

Thanks for the patch @sodonnel . I have just dropped few minor comments. Please take a look. Thanks

.setOmClient(ozoneManagerClient).setRequestID(requestId)
.setReplicationConfig(openKey.getKeyInfo().getReplicationConfig())
.setReplicationConfig((
(ECReplicationConfig)openKey.getKeyInfo().getReplicationConfig()))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: we can remove redundant brackets.

private int ecChunkSize;
private final int numDataBlks;
private final int numParityBlks;
private static final ByteBufferPool BUFFER_POOL = new ElasticByteBufferPool();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Below there is a hardcodec codec name. You may want to take it from replication config?
private static final String DEFAULT_CODEC_NAME = "rs";

Copy link
Copy Markdown
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.

LGTM

@sodonnel sodonnel merged commit 5cf2516 into apache:HDDS-3816-ec Oct 4, 2021
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