[
https://issues.apache.org/jira/browse/HDDS-356?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16587921#comment-16587921
]
Anu Engineer commented on HDDS-356:
-----------------------------------
[~xyao] Thanks for the review comments. Patch v2 addresses most comments.
{quote}Line 62: should we hide the RocksDB.DEFAULT_COLUMN_FAMILY from the
client of this class so that the client only needs to pass the customized
column families or use DBOptions#setCreateMissingColumnFamilies(true); like we
did in RDBStoreTest setup?
{quote}
we will be adding a builder interface in HDDS-359 and that patch will take care
of Defual column support automatically.
{quote}Line 77: Can we wrap the column family options as part of RDBTable
class? This allows do per column family customization later? We will need to
revisit when working on the RockDB tuning per table(column family), can you add
a comment here?
{quote}
You are absolutely correct, I have addressed this also in the Patch that I will
post soon. HDDS-359.
{quote}Line 222: is this only available for rocksDB? Can we get the estimate
per column family(table)?
{quote}
AFAIK, this is an estimate per DB. We can keep a more accurate count in for OM
in the OM code base.
{quote}Line 75-77: this can be removed as the RDBStore#close will close the
DBOptions as well.
{quote}
Good catch. Fixed.
{quote}Line 129: can we add a assert that
{quote}
Fixed.
{quote}Line 59: can you clarify the expected return value when the key is not
found, do we expect a null or an exception?
{quote}
Fixed.
{quote}Line 148/159: can we provide a method that provide key/value without
deep copy?
{quote}
Technically yes, but that would be flagged as security violation by findbugs.
When I read the code first time I investigated this and decided to leave it as
is for time being. It is something that we should do, and silence findbugs on
this issue. I will fix that later in another patch.
{quote}NIT: as a tradition, this should be named TestRDBTableStore, similar
applies to RDBStoreTest.
{quote}
Fixed.
{quote}Line 71-73: this can be removed as the RDBStore#close will close the
DBOptions as well.
{quote}
Fixed.
{quote}Line 148: batch needs to be closed or wrapped with try-with-resource
{quote}
Thanks, good catch. Fixed.
[~nandakumar131] Addressed the offline comment too. In *RDBStore.java*
{code:java}
Families.get(x) replaced with
DFSUtil.bytes2String(columnFamilyHandles.get(x).getName()){code}
> Support ColumnFamily based RockDBStore and TableStore
> -----------------------------------------------------
>
> Key: HDDS-356
> URL: https://issues.apache.org/jira/browse/HDDS-356
> Project: Hadoop Distributed Data Store
> Issue Type: Bug
> Reporter: Xiaoyu Yao
> Assignee: Anu Engineer
> Priority: Major
> Fix For: 0.2.1
>
> Attachments: HDDS-356.001.patch, HDDS-356.002.patch
>
>
> This is to minimize the performance impacts of the expensive RocksDB table
> scan problems from background services disabled by HDDS-355.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
