elek commented on issue #700: HDDS-3172. Use DBStore instead of MetadataStore 
in SCM
URL: https://github.com/apache/hadoop-ozone/pull/700#issuecomment-615289480
 
 
   Thanks @nandakumar131 the review:
   
   > `DBDefinition` is a generic interface which should not have anything 
specific to particular db. It's using `DBStoreBuilder` which is specific to 
rocksdb.
   
   Good point, I didn't notice it. And definition of the format and the 
creation logic were mixed in the new `DBDefinition` interface.
   
   I pushed a new commit (d74528210) where all the creation logic is moved to 
the `DBStoreBuilder`, the `DBDefinition` is a simple DB independent interface. 
(In the future we can also create an interface for `DBStoreBuilder` and create 
a `RDBStoreBuilder` if required.)
   
   > `SCMMetadataStoreRDBImpl` is no more a rocksdb specific implementation. We 
can do the rename/removal in follow-up jira.
   
   Yes. Agree. Do you prefer rename or removal? I am not sure if it's required 
or not.
   
   > I know there is a plan to refactor datanode part, is there a plan to 
re-write OMDBStore to follow this new model of DBDefinition?
   
   Yes. I have two motivation:
   
    * Use only the new DBStore interface everywhere (which would make it easy 
to add upgrade/versioning support) and fully deprecate the old `Metadata` 
interface.
   
    * Re-create the debug tool (`ozone sql` If I remember well) which can 
list/read keys in any DBStore. It requires a unified way to create DB (same 
DBStore should be created from tools and OM/DN/SCM) and maintain the list of 
codecs for each table.
   
   > Can we use ContainerID instead of Long here?
   
   Not sure. Some of the code uses `long` instead of `CotainerId` (eg. 
`updateDeleteTransactionId` is based on long, and `ContainerInfo` is based no 
long). I can use `ContainerId` but it requires some temporary `ContainerId` 
object creation.
   
   I created a commit in this PR to show how can it work (26c8a58ba) check the 
usage of `new ContainerId`. I am fine with both keeping or reverting it. It's 
more typesafe with `ContainerID` and the performance penalty can be very small.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to