Mahadev konar commented on ZOOKEEPER-38:
the patch looks good --- here are some of my comments ---
also I am not finished reviewing it :). its a long patch ---
0) overall there are too many interfaces in the patch. though i uinderstand
that interfaces make things pluggable but I think we should restrict only wehn
we need it and create interfaces only when we seriously think that it would be
good to have it pluggable.
1) DataTereeBuilder is an interface and has only one subclass --- i think that
we should not make this pluggable and just have a datatree class instead of the
interfaces and implementing classes.
2) FileDBINFO implements DBINFO but FILEDBINFO Is the only subclass of DBINFO.
I think here again we should just keep it simple and not make this an interface
3) Snapshotting plicy is again pluggable -- we should also not make this
pluggable and let it work our default implementation.
4) we have arguments passing around as url though there are not true url's in
java -- but just strings. if we really want to make this pluggable and use
url's then we should use URL class in java and use its methods for checking
what kind of provider we want it to return. like create a URL(string) and
url.getProtocol.equals("file") then we retrun our default one.
5) i dont see any use case of more persistence layers as -D expressions? what
if we had something like this
-Dpersisten.provider=classname and then just use reflections to create the
provider class else we just return the default provider. I think it makes the
i am not done reviewing the patch entirely. :)
> headers (version+) in log/snap files
> Key: ZOOKEEPER-38
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-38
> Project: Zookeeper
> Issue Type: New Feature
> Components: server
> Reporter: Patrick Hunt
> Assignee: Andrew Kornev
> Fix For: 3.0.0
> Attachments: ZOOKEEPER-38.patch, ZOOKEEPER-38.patch
> Moved from SourceForge to Apache.
This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.