[
https://issues.apache.org/jira/browse/HDDS-1234?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16799517#comment-16799517
]
Yiqun Lin commented on HDDS-1234:
---------------------------------
Thanks for the explanation, [~swagle] and [~avijayan], very make sense to me.
The latest patch almost looks good, some review comments:
*IntegerCodec*
{code:java}
public byte[] toPersistedFormat(Integer object) throws IOException {
+ return ByteBuffer.allocate(Integer.BYTES).putInt(object).array();
+ }
+
+ @Override
+ public Integer fromPersistedFormat(byte[] rawData) throws IOException {
+ return ByteBuffer.wrap(rawData).getInt();
+ }
{code}
We can simply this as
{{Ints.toByteArray(object);/Ints.fromByteArray(rawData);}} from guava import.
Like {{LongCodec}} did.
*ContainerKeyService*
{code:java}
// Get set of Container-Key mappings for given containerId.
+ for (ContainerKeyPrefix containerKeyPrefix : containerKeyPrefixMap
+ .keySet()) {
+ OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(
+ containerKeyPrefix.getKeyPrefix());
+
{code}
Here maybe we need to do a null check for key info. The table get api only
returns for the specific key. is this a right way we pass a key prefix to get
specific key info? why not prefix match to get?
*OzoneManagerServiceProviderImpl*
The periodic task for obtaining the DB snapshot was removed here but it
doesn't add in {{ContainerKeyMapperTask}}. ContainerKeyMapperTask only does key
reverse mapping frm om snapshot DB. The om snapshot DB should also be updated
periodiclly here I think.
*TestContainerKeyService*
Method name {{getKeysForContainer}} -->{{testGetKeysForContainer}}
*TestContainerDBServiceProviderImpl*
{code:java}
assertEquals(keyPrefixesForContainer.size(), 4);
+ assertEquals(keyPrefixesForContainer.get(ckp2).intValue(), 12);
+ assertEquals(keyPrefixesForContainer.get(ckp3).intValue(), 13);
+ assertEquals(keyPrefixesForContainer.get(ckp4).intValue(), 14);
+ assertEquals(keyPrefixesForContainer.get(ckp5).intValue(), 15);
+
+ assertEquals(containerDbServiceProvider.getCountForForContainerKeyPrefix(
+ ckp1).intValue(), 0);
{code}
The expected value and actul value are passed incorrectly.
*TestContainerKeyMapperTask*
{code:java}
keyPrefixesForContainer =
+ containerDbServiceProvider.getKeyPrefixesForContainer(1);
+ Assert.assertTrue(keyPrefixesForContainer.size() == 1);
+
+ keyPrefixesForContainer =
+ containerDbServiceProvider.getKeyPrefixesForContainer(2);
+ Assert.assertTrue(keyPrefixesForContainer.size() == 1);
{code}
Can we do an addtional detail check for ContainerKeyPrefix in returned
keyPrefixesForContainer?
> Iterate the OM DB snapshot and populate the recon container DB.
> ----------------------------------------------------------------
>
> Key: HDDS-1234
> URL: https://issues.apache.org/jira/browse/HDDS-1234
> Project: Hadoop Distributed Data Store
> Issue Type: Sub-task
> Components: Ozone Recon
> Reporter: Aravindan Vijayan
> Assignee: Aravindan Vijayan
> Priority: Major
> Fix For: 0.5.0
>
> Attachments: HDDS-1234-000.patch, HDDS-1234-001.patch,
> HDDS-1234-002.patch, HDDS-1234-003.patch, HDDS-1234-004.patch
>
>
> * OM DB snapshot contains the Key->ContainerId + BlockId information.
> * Iterate the OM snapshot DB and create the reverse map of (ContainerId, Key
> prefix) -> Key count to be stored in the Recon container DB.
> * Use a codec to store data into Recon container DB.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
