1
Vote

Iterate over all the rows in a ColumnFamily

description

Currently, a query:
            var result = ctx.ColumnList
                .Where(e => e.ColumnFamily == _ColumnFamily);
returns 100 rows. I can increase the number of rows by using Take():
            var result = ctx.ColumnList
                .Where(e => e.ColumnFamily == _ColumnFamily)
                .Take(200);
but this loads all rows into memory before it returns.

Clients can run a loop to iterate over all the rows as stated in Cassandra FAQ:
http://wiki.apache.org/cassandra/FAQ#iter_world
and I'm thinking having this feature built into Cassandraemon is nice.

Thoughts?

If we'd like this change, there are two options:
  1. Change the query without Take() to return all the rows.
  2. Add another property, like CassandraContext.Rows.

1 looks clean, but may have backward compatibility issue.

comments

kojiishi wrote May 25, 2012 at 8:27 PM

Another option:
  1. Set an internal max value of Take(). If Take() exceeds the value, partition the request into mulitple get_range_slices automatically.
Somewhat between #1 and #2; devs must write this to get all rows:
            var result = ctx.ColumnList
                .Where(e => e.ColumnFamily == _ColumnFamily)
                .Take(int.MaxValue)
                .ToList();
and also existing code like Take(10000) may be split into multiple calls so it's not a perfect backward compatible.

sabro wrote May 26, 2012 at 11:30 PM

I am interesting in your idea.

But I don't like #1 because of compatibility issue.
And 100 Row is default of thrift api. I want to go along with it.

2 is against the LINQ rule. Take() is more better.

I like #3. Let's adopt it!
It add internal max value to CassandraConnectionConfig.
Internal max value should be alterable like ConsistencyLevel.

And I have additional request. I want to filter data.
Example, it return 10000 rows with deleteflag == false.

var result = ctx.ColumnList
.Where(e => e.ColumnFamily == _ColumnFamily)
.Where(e => e.Data.First(x => x.Name.ToUTF8() == "deleteflag").Value.ToBool() == false)
.Take(10000)
.ToList();

There may be a much better way.
This feature bring big change to Cassandraemon.
We should discuss it enough.

kojiishi wrote May 27, 2012 at 6:17 PM

Thank you for the comment.

I'm looking into the design, and started thinking that the change is not as large as you may think.

All such cases -- returns too many rows -- fall into get_range_slices or get_indexed_slices, if I'm not mistaken. Do you think this is correct?

And with option #3, I can make the new code path runs only when callers explicitly set the option, so the backward compatibility won't be a big problem.

I can probably start with get_range_slices in a branch and commit for you to review. If you feel good, we can merge that to the default branch. Does this sound good to you?

Also, my project is moving to 1.0 next month, so I'm probably going to make this feature only for 1.0. Please let me know if this isn't a good idea.

kojiishi wrote May 27, 2012 at 6:21 PM

I forgot to mention that, I agree that filters and other LINQ operators should work transparently as in your example.

sabro wrote May 27, 2012 at 11:17 PM

All such cases -- returns too many rows -- fall into get_range_slices or get_indexed_slices, if I'm not mistaken. Do you think this is correct?

And with option #3, I can make the new code path runs only when callers explicitly set the option, so the backward compatibility won't be a big problem.
I understood.
I can probably start with get_range_slices in a branch and commit for you to review. If you feel good, we can merge that to the default branch. Does this sound good to you?
All Right. Please start development in this way.
Also, my project is moving to 1.0 next month, so I'm probably going to make this feature only for 1.0. Please let me know if this isn't a good idea.
OK. I think this feature should be released with Cassandraemon 1.0 & 1.1.

kojiishi wrote May 28, 2012 at 5:28 AM

I've committed in a new branch "workitem37". Can you have a look?

This code supports the feature only for get_range_slices. I was thinking to support multiget_slice too, but a single code to support both operators doesn't look easy, so I gave up the option.

A use case for multiget_slice is, when you have a method that use e.Key.In(string[] keyList), and you need to call the method with, say, 100,000 items in keyList. It just happened to me. I had a method for Web, which processes only a small amount of keys at a time, but then I needed to use it for a batch process too, which requests a large number of rows at once and ended up with Cassandra TimedOutException. I partitioned the array and made a loop to call Cassandraemon, but it's probably nice if Cassandraemon handles such scenario automatically.

get_range_slices is like paging, but for multiget_slice, I think we need to split key array and make multiple requests.

Yet another candidate to implement paging/partitioning is get_indexed_slices. I don't use this operator yet, so I don't have a good idea at this point though. It's probably similar to get_range_slices, but I might need to tweak the code a bit.

So, the proposal is, make this workitem only for get_range_slices. Get your review on the code, fix if any, and close this workitem.

Create two more issues, one for multiget_slice and another for get_range_slices. I probably don't work on them very soon, but we could see if anyone votes on them.

The design of this workitem, along with other two, should keep in mind that the design allows as much code shared between them as possible, and also keep async and Rx support in the horizon.

What do you think?

sabro wrote May 28, 2012 at 7:16 AM

I am worried about CassandraConnectionConfig don't have DefaultMaxRowsPerRequest.

But before then user get confusing to support get_range_slices only.

If we support get_range_slices and not suppot other operation, then user must learn it.
It is not simple. We should implement all or nothing.

I think the best timing of release this feature is the time that all operator supported.

What do you think?

kojiishi wrote May 28, 2012 at 8:32 AM

I am worried about CassandraConnectionConfig don't have DefaultMaxRowsPerRequest.
Yeah, that's a design question whether developers want to set this value per context, or global. I think global settings works better because it's less likely that developers want to change the settings per context.

But you're right that, if we make this a global setting, users can't set it through connection string. Do you think (per context + connection string parameter) works better than a global setting?
But before then user get confusing to support get_range_slices only.
The reason I'm seeing get_range_slices and multiget_slices differently is that, they already behave differently.

When Key is not specified at all, or specified by range (Key > xxx or Key.Between(xxx, yyy) etc.), get_range_slices is used and it returns only 100 rows by default.

When Key.In(...) is used, multiget_slices is used, and it returns all matching rows by default.

This difference actually confused me when I started using Cassandraemon.

Now with the fix, if you use .Take(int.MaxValue), both returns all matching rows, so we're more consistent here than without this fix.

When the result is really huge, get_range_slices works good thanks to automatic partitioning, while multiget_slices will end up with TimedOutException. I agree this part is not consistent.

But the code is so separated that I'm more comfortable to have one workitem for each operator. I'm ok to delay the release until all the workitem are done. Does this sound reasonable?

Also, do you think the work should be done in branch rather than default until all work is done, or are you good to merge to the default branch and keep working on? My concern here is keeping branch for a long time may make the merge more complicated, but since we don't expect many commits to the default branch, maybe either way is fine.

sabro wrote May 28, 2012 at 6:56 PM

But you're right that, if we make this a global setting, users can't set it through connection string. Do you think (per context + connection string parameter) works better than a global setting?
Yes, I think so.
The reason I'm seeing get_range_slices and multiget_slices differently is that, they already behave differently.
Hmm... I can't agree enough because cassandra have also get_indexed_slices.
But the code is so separated that I'm more comfortable to have one workitem for each operator. I'm ok to delay the release until all the workitem are done. Does this sound reasonable?
I'm ok to delay release, too.
Also, do you think the work should be done in branch rather than default until all work is done, or are you good to merge to the default branch and keep working on? My concern here is keeping branch for a long time may make the merge more complicated, but since we don't expect many commits to the default branch, maybe either way is fine.
Cassandra1.1 have new thrift method. So We commit default branch in near future. But probabry new code is not overlap with existing code because of new api. Just to tell you, new api name is get_paged_slice. I have not understood it yet. But it may be able to paging like that we are going to implement. We may have to check that api.

https://issues.apache.org/jira/browse/CASSANDRA-3883

kojiishi wrote May 28, 2012 at 8:33 PM

But you're right that, if we make this a global setting, users can't set it through connection string. Do you think (per context + connection string parameter) works better than a global setting?
Yes, I think so.
Ok, I came up with both-win design. MaxRowsPerRequest is now per context property and is set from ConnectionConfig. Its default value is 0, and if 0, the global (static) CassandraContext.DefaultMaxRowsPerRequest is used.

So dev can set DefaultMaxRowsPerRequest on his/her program, or override it in connection string.

Regarding other operators. The work was much less than I expected, and actually I'm done with all 3 operators; get_range_slices, mutiget_slices, and get_indexed_slices. get_range_slices and get_indexed_slices share the same pager, and there's another pager for multiget_slices.

With this, I think I'm done with this workitem. Could you please have a look at the code and let me know what you think? I've also wrote UT for each operator and it looks working good. I'm more than happy to fix any issues or concerns you found before we merge it to the default branch.

Also, if you have a large diff locally for 1.1, I'm fine to wait for you to commit first and resolve conflicts if any on my side. Please just let me know what you want.
Cassandra1.1 have new thrift method ... new api name is get_paged_slice.
Very interesting, thank you for the information. As far as I quickly read through, it allows column-paging for wide-rows. It also allows row-paging too, so maybe get_range_slice can be deprecated in future.

I personally don't have requirements to page through columns right now, but probably we should get the current paging code work well with get_paged_slice.

sabro wrote May 28, 2012 at 9:46 PM

Ok, I came up with both-win design. MaxRowsPerRequest is now per context property and is set from ConnectionConfig. Its default value is 0, and if 0, the global (static) CassandraContext.DefaultMaxRowsPerRequest is used.
Why don't you use CassandraConnectionConfig.Default.MaxRowsPerRequest like ConsistencyLevel?
Regarding other operators. The work was much less than I expected, and actually I'm done with all 3 operators; get_range_slices, mutiget_slices, and get_indexed_slices. get_range_slices and get_indexed_slices share the same pager, and there's another pager for multiget_slices.
OK. I understood.
Also, if you have a large diff locally for 1.1, I'm fine to wait for you to commit first and resolve conflicts if any on my side. Please just let me know what you want.
I don't have local diff. I implement it after creating secondlife webservice.
Very interesting, thank you for the information. As far as I quickly read through, it allows column-paging for wide-rows. It also allows row-paging too, so maybe get_range_slice can be deprecated in future.
If get_paged_slice allows row-paging, You don't have to implement this feature, don't you?

kojiishi wrote May 29, 2012 at 12:50 PM

Why don't you use CassandraConnectionConfig.Default.MaxRowsPerRequest like ConsistencyLevel?
You're right. I was thinking different thing, but I agree, and fixed.
If get_paged_slice allows row-paging, You don't have to implement this feature, don't you?
Eventually, yes, I think you're right here too. I think paging is a pretty common request in general, and given Cassandra started working on get_paged_slice, it is likely that Cassandra will also implement multiget_paged_slice and get_indexed_paged_slice in future.

It's only about timing. I need the feature now, available for 1.0 (we tend to delay upgrade until stablized,) and I'd also like to use paging for multiget_slice and get_indexed_paged_slice as well.

We don't expose any APIs (other than MaxRowsPerRequest and use of Take,) so I suspect when Cassandra supports request paging in all their APIs, we can move our implementation to it without changing public APIs.

If you don't think such reason is good enough to commit into Cassandraemon, I can keep it out from official releases and move the patch to a fork.

sabro wrote May 29, 2012 at 8:33 PM

Now, We can't know directionality of cassandra.
Thus I want to wait and see for a while.
If you don't think such reason is good enough to commit into Cassandraemon, I can keep it out from official releases and move the patch to a fork.
I think too early to commit main branch. Sorry, please manage in other fork.

kojiishi wrote May 30, 2012 at 6:43 AM

Ok, thank you for having a look and gave me a comment. I'll use the code for my production probably in a week or so, so we might find more issues soon. I'll keep the branch updated if there were any.

Also I'll watch how get_paged_slices goes, but you must be better to catch up information for 1.1. Please keep in touch.