Fix for obvious counting bug in wide row iterator that was counting columns instead of rows.

Several fixes in describe_splits:
fixed non-uniform splitting - caused by integer math roundoff errors
fixed insane behaviour when number of splits was higher than number of key samples
added estimated size of the split to the result, and make use of it in CFIF

This is a patch for broken get_paged_slice; addressed in a separate ticket, but I had to include it in order to test my code

Piotr Kołaczkowski
added a comment - 18/Oct/12 14:48 - edited I attach a list of patches affecting operation of CFRR:
Fix for obvious counting bug in wide row iterator that was counting columns instead of rows.
Several fixes in describe_splits:
fixed non-uniform splitting - caused by integer math roundoff errors
fixed insane behaviour when number of splits was higher than number of key samples
added estimated size of the split to the result, and make use of it in CFIF
This is a patch for broken get_paged_slice; addressed in a separate ticket, but I had to include it in order to test my code
Fix for creating excessively small splits (and wrong progress reporting) due to range wrap around.
get_range_slices allows for (start_key, end_token) exactly the same as get_paged_slice
I tried to de-spaghettize CFRR code a little. This also fixes some bug that accidentally slipped in with previous patches.

Jonathan Ellis
added a comment - 24/Oct/12 20:57 - edited 03 committed in CASSANDRA-4816 .
Not sure about 04 – I'm a fan of the simplifications we get from letting CFRR only need to deal with non-wrapping splits.

#04 - what about virtual nodes in 1.2? Do we insist that split may not span more than one contiguous token range? It will be harder to avoid too small splits. And too small split = bigger task book-keeping overhead.

Piotr Kołaczkowski
added a comment - 05/Nov/12 09:36 #04 - what about virtual nodes in 1.2? Do we insist that split may not span more than one contiguous token range? It will be harder to avoid too small splits. And too small split = bigger task book-keeping overhead.

Hold on with applying patch 2 for a while. We just discovered it breaks running hive queries while doing rolling upgrade. There is a need for falling back to old describe_splits method if describe_splits_ex is not found.

Piotr Kołaczkowski
added a comment - 05/Nov/12 10:13 Hold on with applying patch 2 for a while. We just discovered it breaks running hive queries while doing rolling upgrade. There is a need for falling back to old describe_splits method if describe_splits_ex is not found.

what about virtual nodes in 1.2? Do we insist that split may not span more than one contiguous token range?

That's kind of orthogonal to wrapping ranges per se – you'll still only have a single [virtual] node whose range wraps. So vnodes won't make that worse. Moreover, you're still going to need two scans at the disk level since a wrapping range won't be contiguous there. (Currently wrapping ranges are split by StorageProxy.getRestrictedRanges but this may change for CASSANDRA-4858.) Doing an extra Thrift or CQL query is negligible overhead compared to the actual scan.

Finally, getRestrictedRanges will split it up into scan-per-vnode which I agree is something we should fix but I don't think this patch does it. As an optimization I don't think it's something we should block 1.2.0 for. Should we split this into a separate ticket?

Jonathan Ellis
added a comment - 17/Nov/12 13:02 what about virtual nodes in 1.2? Do we insist that split may not span more than one contiguous token range?
That's kind of orthogonal to wrapping ranges per se – you'll still only have a single [virtual] node whose range wraps. So vnodes won't make that worse. Moreover, you're still going to need two scans at the disk level since a wrapping range won't be contiguous there. (Currently wrapping ranges are split by StorageProxy.getRestrictedRanges but this may change for CASSANDRA-4858 .) Doing an extra Thrift or CQL query is negligible overhead compared to the actual scan.
Finally, getRestrictedRanges will split it up into scan-per-vnode which I agree is something we should fix but I don't think this patch does it. As an optimization I don't think it's something we should block 1.2.0 for. Should we split this into a separate ticket?

Jonathan Ellis
added a comment - 17/Nov/12 13:02 0006: Can you split out the bug fix and rebase? Refactoring is fine but let's keep it separate from bug fixes.
0007: I'm unclear what this is useful for, anyone running a 1.1 recent enough to have this patch, would also have describe_splits_ex, no?

Piotr Kołaczkowski
added a comment - 20/Nov/12 15:48 0007 - this is for rolling upgrade. When you upgrade one node and the other nodes don't have describe_splits_ex yet, starting a hadoop job on a newly upgraded node fails.
As for 0004 / 0006 fixes - I agree. Let's move them to a separate ticket.

BTW: I don't get email notifications from ASF Jira. It is because I registered long, long time ago and my email is obsolete. How to change my email to a newer one? I can't see an option in the user profile for doing that.

Piotr Kołaczkowski
added a comment - 20/Nov/12 15:49 BTW: I don't get email notifications from ASF Jira. It is because I registered long, long time ago and my email is obsolete. How to change my email to a newer one? I can't see an option in the user profile for doing that.