Skip to content

Conversation

@rakeshadr
Copy link
Contributor

What changes were proposed in this pull request?

This task is to use open file table for the allocate block operations. This has been identified as part of Hive TPCDS benchmark test.

What is the link to the Apache JIRA

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

How was this patch tested?

Added UT cases. Also, verified by running hive tpcds test

Copy link
Contributor

@linyiqun linyiqun left a comment

Choose a reason for hiding this comment

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

The change looks good to me. Below some minor comments from me.

}
Assert.assertFalse("key already exists /root_dir/file1",
fs.rename(abcRootPath, file1Destin));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Above rename related change already contained in PR #1607.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@linyiqun yes, you are right. Since that is in progress, I just added to make clean build report. We can push #1607 push/upstream it first and then I will rebase this PR on top of that. Hope that would be fine.

*/
public class TestOMAllocateBlockResponseV1
extends TestOMAllocateBlockResponse {

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to override getOzoneConfiguration method in this class?

  protected OzoneConfiguration getOzoneConfiguration() {
    OzoneConfiguration config = super.getOzoneConfiguration();
    config.set(OMConfigKeys.OZONE_OM_LAYOUT_VERSION, "V1");
    return config;
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @linyiqun , configuring V1 version will be required to pick new tables, otherwise all the db entries will go to old table with new format.

In new PR #1679, I've updated existing response test cases as well to follow same pattern. Please review it again. Thanks!

@bharatviswa504
Copy link
Contributor

@rakeshadr can you rebase this PR, it has multiple changes looks like it.
Can you post a PR related to this Jira, which will make it easier to review?

@rakeshadr
Copy link
Contributor Author

Current PR has multiple changes and looks like stale. I am closing this.
@linyiqun @bharatviswa504 , I've created new PR# #1679 on top of the branch. Kindly review the same. Thanks!

@rakeshadr rakeshadr closed this Dec 9, 2020
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.

3 participants