elek commented on pull request #1298: URL: https://github.com/apache/hadoop-ozone/pull/1298#issuecomment-687089838
Thanks @errose28 the patch. I didn't check it fully, but I tried it with creating a new [half-baked patch](https://github.com/apache/hadoop-ozone/commit/80c8ecf05b310cc05aa148a1075aea8dbd763124) to try if we can support multiple RocksDB scheme (shared / per-container). I really liked the approach: Using wrapper `Table<>` implementation to add prefixes (like in `SchemaOneDeletedBlocksTable`) Also, `DatanodeStore` interface provides a good interface which can be re-used in the future (I removed `start/stop` from the interface, but in this patch it's fine). Earlier I realized that the interfaces of the containers (like `ContainerData`) is not very straightforward. I think we can use one generic `ContainerData` implementation long-term (instead of casting...), but -- again -- this is not for this patch. But I would consider moving `schemaVersion` to `ContainerData` as it seems to be a generic (and important) field (not blocking, either). +1 The high level class hierarchy, concept looks good to me (but I am not very good to find low level problems, others are more attentive than me ;-) ) ---------------------------------------------------------------- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
