You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/01/12 18:55:20 UTC
[GitHub] [ozone] JyotinderSingh opened a new pull request #2983: HDDS-3408. Rename ChunkLayOutVersion to ContainerLayoutVersion.
JyotinderSingh opened a new pull request #2983:
URL: https://github.com/apache/ozone/pull/2983
## What changes were proposed in this pull request?
Because this layOutversion defines/handles any changes related to chunks/blocks/container data. So, this layOutVersion on whole handles the entire Container. This jira proposes to rename ChunkLayOutVersion -> ContainerLayoutVersion. In this way, it will provide a clear explanation of this layOutVersion field.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3408
## How was this patch tested?
Related integration and unit tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JyotinderSingh commented on pull request #2983: HDDS-3408. Rename ChunkLayOutVersion to ContainerLayoutVersion.
Posted by GitBox <gi...@apache.org>.
JyotinderSingh commented on pull request #2983:
URL: https://github.com/apache/ozone/pull/2983#issuecomment-1011355634
Hi @bharatviswa504 I found this unassigned Jira created by you and decided to take it up. Is this patch still required?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a change in pull request #2983: HDDS-3408. Rename ChunkLayOutVersion to ContainerLayoutVersion.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2983:
URL: https://github.com/apache/ozone/pull/2983#discussion_r785050062
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerData.java
##########
@@ -225,11 +226,11 @@ public long getMaxSize() {
}
/**
- * Returns the layOutVersion of the actual container data format.
- * @return layOutVersion
+o * Returns the layoutVersion of the actual container data format.
Review comment:
Stray character:
```suggestion
* Returns the layoutVersion of the actual container data format.
```
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerData.java
##########
@@ -129,19 +129,20 @@
* Creates a ContainerData Object, which holds metadata of the container.
* @param type - ContainerType
* @param containerId - ContainerId
- * @param layOutVersion - Container layOutVersion
+ * @param layoutVersion - Container layoutVersion
* @param size - Container maximum size in bytes
* @param originPipelineId - Pipeline Id where this container is/was created
* @param originNodeId - Node Id where this container is/was created
*/
protected ContainerData(ContainerType type, long containerId,
- ChunkLayOutVersion layOutVersion, long size, String originPipelineId,
- String originNodeId) {
+ ContainerLayoutVersion layoutVersion, long size,
+ String originPipelineId,
+ String originNodeId) {
Preconditions.checkNotNull(type);
this.containerType = type;
this.containerID = containerId;
- this.layOutVersion = layOutVersion.getVersion();
+ this.layOutVersion = layoutVersion.getVersion();
Review comment:
Nit: Please also rename `this.layOutVersion` to `this.layoutVersion`.
Also in `DatanodeVersionFile`.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerDataYaml.java
##########
@@ -252,8 +252,9 @@ public Object construct(Node node) {
//Needed this, as TAG.INT type is by default converted to Long.
long layOutVersion = (long) nodes.get(OzoneConsts.LAYOUTVERSION);
Review comment:
Nit: for consistency.
```suggestion
long layoutVersion = (long) nodes.get(OzoneConsts.LAYOUTVERSION);
```
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerDataYaml.java
##########
@@ -252,8 +252,9 @@ public Object construct(Node node) {
//Needed this, as TAG.INT type is by default converted to Long.
long layOutVersion = (long) nodes.get(OzoneConsts.LAYOUTVERSION);
- ChunkLayOutVersion layoutVersion =
- ChunkLayOutVersion.getChunkLayOutVersion((int) layOutVersion);
+ ContainerLayoutVersion layoutVersion =
+ ContainerLayoutVersion.getContainerLayoutVersion(
+ (int) layOutVersion);
Review comment:
Nit: for consistency.
```suggestion
(int) layoutVersion);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JyotinderSingh commented on a change in pull request #2983: HDDS-3408. Rename ChunkLayOutVersion to ContainerLayoutVersion.
Posted by GitBox <gi...@apache.org>.
JyotinderSingh commented on a change in pull request #2983:
URL: https://github.com/apache/ozone/pull/2983#discussion_r785079130
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerData.java
##########
@@ -129,19 +129,20 @@
* Creates a ContainerData Object, which holds metadata of the container.
* @param type - ContainerType
* @param containerId - ContainerId
- * @param layOutVersion - Container layOutVersion
+ * @param layoutVersion - Container layoutVersion
* @param size - Container maximum size in bytes
* @param originPipelineId - Pipeline Id where this container is/was created
* @param originNodeId - Node Id where this container is/was created
*/
protected ContainerData(ContainerType type, long containerId,
- ChunkLayOutVersion layOutVersion, long size, String originPipelineId,
- String originNodeId) {
+ ContainerLayoutVersion layoutVersion, long size,
+ String originPipelineId,
+ String originNodeId) {
Preconditions.checkNotNull(type);
this.containerType = type;
this.containerID = containerId;
- this.layOutVersion = layOutVersion.getVersion();
+ this.layOutVersion = layoutVersion.getVersion();
Review comment:
The variable name `layOutVersion` is tightly coupled with the name of the key inside the container data YAML file. Renaming this to `layoutVersion` will change the key persisted into the YAML to `"layoutVersion"` as well (potentially breaking backward compatibility).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] errose28 merged pull request #2983: HDDS-3408. Rename ChunkLayOutVersion to ContainerLayoutVersion.
Posted by GitBox <gi...@apache.org>.
errose28 merged pull request #2983:
URL: https://github.com/apache/ozone/pull/2983
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JyotinderSingh commented on a change in pull request #2983: HDDS-3408. Rename ChunkLayOutVersion to ContainerLayoutVersion.
Posted by GitBox <gi...@apache.org>.
JyotinderSingh commented on a change in pull request #2983:
URL: https://github.com/apache/ozone/pull/2983#discussion_r783398661
##########
File path: hadoop-hdds/common/src/main/resources/ozone-default.xml
##########
@@ -722,11 +722,11 @@
</description>
</property>
<property>
- <name>ozone.scm.chunk.layout</name>
+ <name>ozone.scm.container.layout</name>
Review comment:
Thank you for letting me know this, I have addressed this issue in my latest commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a change in pull request #2983: HDDS-3408. Rename ChunkLayOutVersion to ContainerLayoutVersion.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2983:
URL: https://github.com/apache/ozone/pull/2983#discussion_r785280553
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerData.java
##########
@@ -129,19 +129,20 @@
* Creates a ContainerData Object, which holds metadata of the container.
* @param type - ContainerType
* @param containerId - ContainerId
- * @param layOutVersion - Container layOutVersion
+ * @param layoutVersion - Container layoutVersion
* @param size - Container maximum size in bytes
* @param originPipelineId - Pipeline Id where this container is/was created
* @param originNodeId - Node Id where this container is/was created
*/
protected ContainerData(ContainerType type, long containerId,
- ChunkLayOutVersion layOutVersion, long size, String originPipelineId,
- String originNodeId) {
+ ContainerLayoutVersion layoutVersion, long size,
+ String originPipelineId,
+ String originNodeId) {
Preconditions.checkNotNull(type);
this.containerType = type;
this.containerID = containerId;
- this.layOutVersion = layOutVersion.getVersion();
+ this.layOutVersion = layoutVersion.getVersion();
Review comment:
You are right, I forgot about that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JyotinderSingh commented on a change in pull request #2983: HDDS-3408. Rename ChunkLayOutVersion to ContainerLayoutVersion.
Posted by GitBox <gi...@apache.org>.
JyotinderSingh commented on a change in pull request #2983:
URL: https://github.com/apache/ozone/pull/2983#discussion_r785079130
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerData.java
##########
@@ -129,19 +129,20 @@
* Creates a ContainerData Object, which holds metadata of the container.
* @param type - ContainerType
* @param containerId - ContainerId
- * @param layOutVersion - Container layOutVersion
+ * @param layoutVersion - Container layoutVersion
* @param size - Container maximum size in bytes
* @param originPipelineId - Pipeline Id where this container is/was created
* @param originNodeId - Node Id where this container is/was created
*/
protected ContainerData(ContainerType type, long containerId,
- ChunkLayOutVersion layOutVersion, long size, String originPipelineId,
- String originNodeId) {
+ ContainerLayoutVersion layoutVersion, long size,
+ String originPipelineId,
+ String originNodeId) {
Preconditions.checkNotNull(type);
this.containerType = type;
this.containerID = containerId;
- this.layOutVersion = layOutVersion.getVersion();
+ this.layOutVersion = layoutVersion.getVersion();
Review comment:
The variable name `layOutVersion` itself is used to name the key inside the container data YAML file. Renaming this to `layoutVersion` will change the key persisted into the YAML to `"layoutVersion"` as well (potentially breaking backward compatibility).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a change in pull request #2983: HDDS-3408. Rename ChunkLayOutVersion to ContainerLayoutVersion.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2983:
URL: https://github.com/apache/ozone/pull/2983#discussion_r783380818
##########
File path: hadoop-hdds/common/src/main/resources/ozone-default.xml
##########
@@ -722,11 +722,11 @@
</description>
</property>
<property>
- <name>ozone.scm.chunk.layout</name>
+ <name>ozone.scm.container.layout</name>
Review comment:
Changing config property names needs to be backward compatible by linking the old and new ones:
https://github.com/apache/ozone/blob/d09cdd4a585b66e95f32c8a054ee493101ee4de0/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/conf/OzoneConfiguration.java#L299-L309
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 commented on pull request #2983: HDDS-3408. Rename ChunkLayOutVersion to ContainerLayoutVersion.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #2983:
URL: https://github.com/apache/ozone/pull/2983#issuecomment-1011384933
> Hi @bharatviswa504 I found this unassigned Jira created by you and decided to take it up. Is this patch still required?
@JyotinderSingh It is good to have, but as it involves config change we need to handle it in a backward compatible manner.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JyotinderSingh commented on pull request #2983: HDDS-3408. Rename ChunkLayOutVersion to ContainerLayoutVersion.
Posted by GitBox <gi...@apache.org>.
JyotinderSingh commented on pull request #2983:
URL: https://github.com/apache/ozone/pull/2983#issuecomment-1011399548
Thank you for the reviews @adoroszlai @bharatviswa504. I have linked the old config key to the new one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org