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

Move Operation from Keyspace to CassandraClient #24

Closed
hz opened this issue Apr 8, 2010 · 6 comments
Closed

Move Operation from Keyspace to CassandraClient #24

hz opened this issue Apr 8, 2010 · 6 comments

Comments

@hz
Copy link

hz commented Apr 8, 2010

Move operateWithFailover and operateWithFailoverSingleIteration from KeyspaceImpl to CassandraClientImpl. Leave keyspace with a clear skelton to package the thrift api.
If the users don't want to use KeyspaceImpl, they can use CassandraClientImpl still.

@zznate
Copy link
Collaborator

zznate commented Apr 8, 2010

Within hector, the read/write operations all happen in Keyspace, CassandraClient encapsulates the connection to an individual Cassandra instance. Failover in this context is within the data operation on the inherently distributed keyspace, so those methods live there. In other words, we fail over to another client from within a keyspace operation, not vice versa.

If you choose to get a handle to the Cassandra.Client thrift stuff from CassandraClient, then you are loosing the functionality in Keyspace and to a large degree are on your own anyway.

@rantav
Copy link
Collaborator

rantav commented Apr 8, 2010

@hz your point is in place and this is a design issue I've been uncomfortable with from the beginning, but I agree with @zznate that the solution you suggest isn't going to work, at least no as things are now.

If you want to use Keyspace without failover you have 1 choice currently, to use FAIL_FAST.
Still you'd incur the (small) overhead of creating a client etc, but logically I believe it gets you what you need. I agree that this is not as clean, from the API perspective, but logically it's the same.

What we can do, though is maybe to implement two Keyspace implementations - one SimpleKeyspace and one FailoverCapableKeyspace and let the clients instantiate whichever Keyspace flavor they prefer.

I see this as a code enhancement, not a real feature. Let us know what you think.

Also, if you need this badly, we love contributions, just fork the project, make your changes and ping us for a code review.

@hz
Copy link
Author

hz commented Apr 8, 2010

I had forked one before I created this issue. Let me think more about it.

@hz
Copy link
Author

hz commented Apr 8, 2010

So, before any result is reached, I close this issue firstly. :)

@hz
Copy link
Author

hz commented Apr 9, 2010

I added one interface: HectorRequest, to package the failover and pooling features.
The implements HectorRequestImpl implements the operation and failover.
The third class is HectorThriftRequest which extends HectorRequestImpl. It package all the thrift api in it.
Keyspace now initiate with a hector request. Each failover operation is run as request.thrift_api(parameter).
But you do some tricks on getColumn. I don't understand its logic.
I am not investigate the failover logic you used. So the code have many many mistakes. Please take some time to browse my code and decide if you can merge some into your trunk.
Thanks.

@rantav
Copy link
Collaborator

rantav commented Apr 11, 2010

Hi @hz, I'm sorry I had browsed http://github.com/hz/hector and couldn't find your changes. Could you post here (or email the dev mailing list from http://wiki.github.com/rantav/hector/mailing-lists) a link to your commits (something like http://github.com/rantav/hector/commit/4321125f79e9c3198ea2116527cd07546d737483)

Thanks!

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

No branches or pull requests

3 participants