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

Extending MemcacheClientBuilder to more easily allow other AbstractMultiMemcacheClient impl #198

Open
jreilly650 opened this issue Mar 31, 2022 · 2 comments

Comments

@jreilly650
Copy link
Contributor

Hi,
I am currently looking at using folsom. One thing I want to do it to implement my own sharding (to match existing impl). To do this, I was planning to implement a class extending AbstractMultiMemcacheClient similar to KetamaMemcacheClient.

Initially I was planning to extend MemcacheClientBuilder, noticing that the MemcacheClientBuilder.connectRaw method is protected, however on closer inspection I noticed that many fields that one might want to use in that method (such as this.addresses) are not accessible. Would you have any objection to a patch that changes the current set of fields from private to protected?

Another possible alternative I was thinking about is addition of a method to the builder that allows one to pass in the code to create an AbstractMultiMemcacheClient given a list of addresses. e.g.

MemcacheClientBuilder<V> withRawClientCreator(Function<List<HostAndPort>, RawMemcacheClient> clientCreationFunction)

Then in connectRaw, use the function if it is not null, otherwise use the existing logic. Either something like this, or being able to extend and override connectRaw would work for me, but I'm wondering if there would be any interest in a solution like withRawClientCreator? Let me know what you think.

Thanks!

@spkrka
Copy link
Member

spkrka commented Apr 7, 2022

Thanks for the issue!

For me it would be easiest to think about this once the PR is prepared for review. It sounds reasonable to me now, but perhaps there is some detail that makes it preferable to do it in some other way.

I think in general it seems useful to think of MemcacheClientBuilder is a convenience structure, but it should be possible to set up the client in a more customized way. Exactly how to do that, I am still a bit undecided.

If the PR is not too bothersome to create, let's start with that, otherwise I can spend some more time to think about it.

@jreilly650
Copy link
Contributor Author

Yeah, that makes sense. I'll put together a PR as soon as I can.

Cheers!

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