Skip to content

Conversation

@rakeshadr
Copy link
Contributor

What changes were proposed in this pull request?

This task is to improve the key location updation logic during commit block.

What is the link to the Apache JIRA

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

How was this patch tested?

Added UT.

@rakeshadr rakeshadr requested a review from xiaoyuyao April 5, 2021 17:44
@elek elek self-requested a review April 12, 2021 16:28
Copy link
Contributor

@xiaoyuyao xiaoyuyao left a comment

Choose a reason for hiding this comment

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

Thanks @rakeshadr for working on this. The PR lGTM overall. A few comments added inline...

Copy link
Member

@elek elek left a comment

Choose a reason for hiding this comment

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

+1, thanks the fix @rakeshadr (one minor typo in UnKnown)

I tested it with a custom freon test which tried to create a new key with a blockid which is not allocated earlier:

bash-4.2$ kinit -kt /etc/security/keytabs/scm.keytab scm/scm
bash-4.2$ ozone sh volume create /vol1 
bash-4.2$ ozone sh bucket create /vol1/bucket1^C
bash-4.2$ ozone sh volume create /vol1 
bash-4.2$ ozone sh bucket create /vol1/bucket1
bash-4.2$ ozone sh bucket create /vol1/bucket2
bash-4.2$ ozone sh bucket setacl -a user:testuser/[email protected]:a /vol1/bucket2
ACLs set successfully.
bash-4.2$ ozone sh volume setacl -a user:testuser/[email protected]:a /vol1
ACLs set successfully.
bash-4.2$ ozone sh key put /vol1/bucket1/key1 README.md 
bash-4.2$ ozone sh key info /vol1/bucket1/key1 | jq -r '.ozoneKeyLocations[0] | .localID'
107544261427200000

Now using the blockId which is allocated for key1 I tried to create a new key:

bash-4.2$ kdestroy
bash-4.2$ kinit -kt /ect/security/keytabs/testuser.keytab testuser/scm
bash-4.2$ kinit -kt /etc/security/keytabs/testuser.keytab testuser/scm
bash-4.2$ ozone sh --verbose btpc --block-id 107544261427200000 --om om

106097636080746497

key cretead /vol1/bucket2/key663262114
bash-4.2$ ozone sh --verbose btpc --block-id 107544261427200000 --om om

106097641685188610

key cretead /vol1/bucket2/key1174965481

The keys are created but it clearly visible that the key location is updated to the original one:

bash-4.2$ ozone sh key cat /vol1/bucket2/key1174965481

It's an empty fie, the content of the original block (README.md) is not visible.

(source of the the freon test is at https://github.com/elek/ozone/tree/block-token-test)

Co-authored-by: Elek, Márton <[email protected]>
@rakeshadr
Copy link
Contributor Author

+1, thanks the fix @rakeshadr (one minor typo in UnKnown)

I tested it with a custom freon test which tried to create a new key with a blockid which is not allocated earlier:

bash-4.2$ kinit -kt /etc/security/keytabs/scm.keytab scm/scm
bash-4.2$ ozone sh volume create /vol1 
bash-4.2$ ozone sh bucket create /vol1/bucket1^C
bash-4.2$ ozone sh volume create /vol1 
bash-4.2$ ozone sh bucket create /vol1/bucket1
bash-4.2$ ozone sh bucket create /vol1/bucket2
bash-4.2$ ozone sh bucket setacl -a user:testuser/[email protected]:a /vol1/bucket2
ACLs set successfully.
bash-4.2$ ozone sh volume setacl -a user:testuser/[email protected]:a /vol1
ACLs set successfully.
bash-4.2$ ozone sh key put /vol1/bucket1/key1 README.md 
bash-4.2$ ozone sh key info /vol1/bucket1/key1 | jq -r '.ozoneKeyLocations[0] | .localID'
107544261427200000

Now using the blockId which is allocated for key1 I tried to create a new key:

bash-4.2$ kdestroy
bash-4.2$ kinit -kt /ect/security/keytabs/testuser.keytab testuser/scm
bash-4.2$ kinit -kt /etc/security/keytabs/testuser.keytab testuser/scm
bash-4.2$ ozone sh --verbose btpc --block-id 107544261427200000 --om om

106097636080746497

key cretead /vol1/bucket2/key663262114
bash-4.2$ ozone sh --verbose btpc --block-id 107544261427200000 --om om

106097641685188610

key cretead /vol1/bucket2/key1174965481

The keys are created but it clearly visible that the key location is updated to the original one:

bash-4.2$ ozone sh key cat /vol1/bucket2/key1174965481

It's an empty fie, the content of the original block (README.md) is not visible.

(source of the the freon test is at https://github.com/elek/ozone/tree/block-token-test)

Thanks @elek for the test result. I have committed the typo suggestion.

Copy link
Contributor

@bharatviswa504 bharatviswa504 left a comment

Choose a reason for hiding this comment

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

+1 LGTM

@rakeshadr
Copy link
Contributor Author

Thanks @xiaoyuyao, @elek, @bharatviswa504 for the reviews and for the approval. I'll merge this to the master branch shortly.

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.

4 participants