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/04/30 14:56:20 UTC
[GitHub] [ozone] kaijchen opened a new pull request, #3371: HDDS-6676. KeyValueContainerData#getProtoBufMessage() should set block count
kaijchen opened a new pull request, #3371:
URL: https://github.com/apache/ozone/pull/3371
## What changes were proposed in this pull request?
When implementing HDDS-6518, I found `KeyValueContainerData#getProtoBufMessage()` does not set block count.
Although it is an optional field in `ContainerData`, I think we should set it to make `ReadContainer` command more useful.
HDDS-6518 may not get merged since another approach HDDS-6665 is taken. So I filed a separate Jira for this improvement.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6676
## How was this patch tested?
See `TestReplicationService#testReadContainer` in #3351.
But I don't think there is a need to add new test in this PR.
--
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] kerneltime commented on pull request #3371: HDDS-6676. KeyValueContainerData#getProtoBufMessage() should set block count
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3371:
URL: https://github.com/apache/ozone/pull/3371#issuecomment-1115335416
Can we add a unit test where all attributes unless explicitly skipped are part of the serialized protobuf?
--
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 merged pull request #3371: HDDS-6676. KeyValueContainerData#getProtoBufMessage() should set block count
Posted by GitBox <gi...@apache.org>.
adoroszlai merged PR #3371:
URL: https://github.com/apache/ozone/pull/3371
--
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] kaijchen commented on pull request #3371: HDDS-6676. KeyValueContainerData#getProtoBufMessage() should set block count
Posted by GitBox <gi...@apache.org>.
kaijchen commented on PR #3371:
URL: https://github.com/apache/ozone/pull/3371#issuecomment-1171843360
Thanks @kerneltime @neils-dev and @adoroszlai for the review.
--
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] kaijchen commented on pull request #3371: HDDS-6676. KeyValueContainerData#getProtoBufMessage() should set block count
Posted by GitBox <gi...@apache.org>.
kaijchen commented on PR #3371:
URL: https://github.com/apache/ozone/pull/3371#issuecomment-1124556250
Hi @neils-dev, thanks for the advice. I have added the test.
--
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] neils-dev commented on pull request #3371: HDDS-6676. KeyValueContainerData#getProtoBufMessage() should set block count
Posted by GitBox <gi...@apache.org>.
neils-dev commented on PR #3371:
URL: https://github.com/apache/ozone/pull/3371#issuecomment-1124520669
A unit test can be added to `TestKeyValueContainer` (_hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java_). This test, `TestKeyValueContainer#testKeyValueDataProtoBufMsg`, can exercise the protobuf and all attributes set:
ie.
```
populate(numberOfKeysToWrite);
ContainerDataProto proto = containerData.getProtoBufMessage();
assertEquals(proto.getState(), keyValueContainerData.getState());
...
assertEquals(proto.getBlockCount(), numberOfKeysToWrite);
```
--
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 pull request #3371: HDDS-6676. KeyValueContainerData#getProtoBufMessage() should set block count
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #3371:
URL: https://github.com/apache/ozone/pull/3371#issuecomment-1171647607
Thanks @kaijchen for the patch, @kerneltime and @neils-dev for the review.
--
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] kaijchen commented on pull request #3371: HDDS-6676. KeyValueContainerData#getProtoBufMessage() should set block count
Posted by GitBox <gi...@apache.org>.
kaijchen commented on PR #3371:
URL: https://github.com/apache/ozone/pull/3371#issuecomment-1115584147
> Can we add a unit test where all attributes unless explicitly skipped are part of the serialized protobuf?
I think we can check `x.equals(x.fromProto(x.toProto))`, do you have any better idea?
--
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