HDDS-10319. Also consider bucket layout deciding whether to normalize path for listKeys

[devmadhuu]

What changes were proposed in this pull request?

This PR is to fix normalization of keyPrefix path based on bucket layout. We have "ozone.om.enable.filesystem.paths" as false by default. This config is currently global irrespective of any check for bucket layout. Ideally this config should be checked along with bucket layout because being a global config, path normalization logic should not be applicable for all bucket types. Config makes sense only for LGEACY bucket types only.

When we have OBS or LEGACY bucket having keyPrefix starting with "/" like "/readPath/" and fsPath config ("ozone.om.enable.filesystem.paths" ) is enabled (true), then code flow will hit normalization of key during org.apache.hadoop.ozone.om.KeyManagerImpl#listKeys API call flow and normalized key will be "vol/buck/readPath", but in keyTable, key will be saved as "vol/buck//readPath/", so it doesn't match and listKeys API wouldn't be able to retrieve the key with normalized key path.
Test org.apache.hadoop.ozone.freon.TestOmBucketReadWriteKeyOps.testOmBucketReadWriteKeyOps was failing when ozone.om.enable.filesystem.paths was set to true.

What is the link to the Apache JIRA

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

How was this patch tested?

Patch was tested using existing Junit and integration tests.

[devmadhuu]

@sumitagrawl @sadanand48 Pls review.

[tanvipenumudy]

Thank you @devmadhuu for the patch. I have a few minor nits.

I think it might also help to have parameterization around any of the existing testcases for the configuration: ozone.om.enable.filesystem.paths (if possible) - if we do not have one already, thanks.

[tanvipenumudy]

This line seems unintentional, could you please address this if not needed?

[devmadhuu]

Thanks @tanvipenumudy for review. I have removed the commented code.

[sadanand48]

Thanks @devmadhuu for the fix, Was this issue causing any tests to fail? Is there a unit test which verifies this?

[devmadhuu]

Thank you @devmadhuu for the patch. I have a few minor nits.

I think it might also help to have parameterization around any of the existing testcases for the configuration: ozone.om.enable.filesystem.paths (if possible) - if we do not have one already, thanks.

Since we have not added any new configuration, existing test case are already covering it.

[devmadhuu]

Thanks @devmadhuu for the fix, Was this issue causing any tests to fail? Is there a unit test which verifies this?

Thanks @sadanand48 for the review. Yes an existing Test org.apache.hadoop.ozone.freon.TestOmBucketReadWriteKeyOps.testOmBucketReadWriteKeyOps was failing when ozone.om.enable.filesystem.paths was set to true. And other test of similar nature will fail if config set to true when keyPrefix is having leading trailing slash.

[sadanand48]

Yes an existing Test org.apache.hadoop.ozone.freon.TestOmBucketReadWriteKeyOps.testOmBucketReadWriteKeyOps was failin

Can we make it parameterized to have a test for this fix . something like below and set config based on the boolean

@ParameterizedTest(name = "Filesystem Paths Enabled: {0}")
@ValueSource(booleans = {false, true})
public void testOmBucketReadWriteKeyOps(boolean fsPathsEnabled) throws Exception {
  try {
    startCluster(fsPathsEnabled);

[devmadhuu]

fsPathsEnabled

Yes, its added now. Pls re-review.

[sadanand48]

Thanks @devmadhuu for addressing comments . LGTM

[adoroszlai]

Thanks @devmadhuu for the fix, @sadanand48, @tanvipenumudy for the review.