[GitHub] [hadoop-ozone] elek commented on issue #700: HDDS-3172. Use DBStore instead of MetadataStore in SCM

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org