Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OWNER seems to use Properties#entrySet() internally. Any way to change that? #198

Open
StFS opened this issue Feb 14, 2017 · 3 comments
Open

Comments

@StFS
Copy link
Contributor

StFS commented Feb 14, 2017

@lviggiano this is just a question about the internal design and implementation of OWNER.

The "back story" so to speak, is that I was thinking about implementing a loader that would be backed by etcd and maybe even another one backed by redis. I took a look at the ZooKeeperLoader and must admit that I don't quite understand the way things are done there. Why is there a full blown URI handler being registered (only to have the openStream() method return an instance that isn't really an input stream at all)?

But that really isn't my main concern. I was thinking about creating an "etcd/redis backed Properties" class. The idea being that you could construct one of those and register it as a source for the ConfigFactory and when a Config interface was requesting a property with a certain key (say "foo.bar"), the RedisProperties class would get asked for it and it could simply delegate the request to Redis.

However, when I started looking into this, I noticed that OWNER seems to be calling "entrySet()" on the Properties instances that it uses to provide values. This causes a major problem for things like etcd and Redis (and ZooKeeper for that matter as well) because you do not want to request all the keys from a service like that. It may be storing a huge amount of keys that have nothing to do with the config properties that you're interested in.

So my question is: is there a reason for this design or could it possibly be changed so that only keys that are actually needed are requested? Requesting the key values as needed would be better (these calls could be delegated one at a time) but I think that if there were some way for OWNER to "inform" what keys will be requested, that would be even better. That way, a single "batch" request could be made to retrieve all the necessary values initially and the results could be cached.

I'd be more than happy to do some of the work for this but I just don't feel like I'm familiar enough with the core design and implementation to jump right in. At the very least I'd appreciate some feedback on if you think this would be doable at all before I start dipping in.

@StFS
Copy link
Contributor Author

StFS commented Feb 14, 2017

I should mention that currently, owner also uses Properties#keySet() but that is actually my own fault since I added that code in a recent PR to ensure that there were no null values in the maps that were being supplied to provide the key values. That would have to be refactored as well.

@StFS
Copy link
Contributor Author

StFS commented Apr 6, 2017

@lviggiano I see you've been active in responding to issues here recently, I'd really love to get your input on this. The main question I have for you is is there any way to avoid calling entrySet() on all Properties classes in OWNER?

@shakedel
Copy link

Can you give a pointer to the Properties#keySet() that gives you trouble

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants