cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sylvain Lebresne (JIRA)" <>
Subject [jira] [Commented] (CASSANDRA-9893) Fix upgrade tests from #9704 that are still failing
Date Wed, 02 Sep 2015 15:59:46 GMT


Sylvain Lebresne commented on CASSANDRA-9893:

Thanks for the rebase.

bq. CASSANDRA-9554 fixed edge_2i_on_complex_pk_test and indexed_with_eq_test.

That's a little bit surprising to be honest, but as long as none of us see failures with those
tests, I'll take it.

bq. {{composite_index_collections_test}} is still failing for me on cassandra-3.0 though

This is weird because I'm still not seeing this (I just ran this test 5 times without issue).

I also remember why we pass {{false}} to the {{DataRange.forPaging()}} call in that case:
that's because what 2.1 sends for the {{startBound}} will be the name of the _last_ cell (let's
call it {{C}}) for the last row (let's call it {{R}}) that was returned. On 3.0 however, we
transform that cell name into a row clustering, so if we include it, we'll select the whole
row {{R}}. When we send it back to 2.1 however, the node will expect to see {{C}} as first
thing in the result, but instead they will get the first cell of {{R}} and we'll re-include
{{R}} even though it was previously returned, hence duplicating a result. So as far as I can
tell, using {{false}} unconditionally is the right thing to do here.

I'll add that I'm not sure how the change of the first commit could ever fix {{composite_index_collections_test}}.
The query of that test doesn't have any condition on the clustering columns and all results
will be returned in the first page (since there is so few of them). So the (only) query sent
from the 2.1 node will have its {{start}} be empty (see {{RangeSliceQueryPager.queryNextPage()}}
in 2.1/2.2. On the first page, {{start}} is assigned the start of the {{SliceQueryFilter}},
and because there is not clustering columns condition, this will be {{Composites.EMPTY}}).
As a result, when deserializing that command on the 3.0 node (in {{ReadCommand.LegacyPagedRangeCommandSerializer.deserialize}}),
the {{startBound}} variable will be {{LegacyLayout.LegacyBound.BOTTOM}} (or it's a bug) and
the line the first commit changes won't even be used.

bq. {{cql_tests:TestCQL.static_columns_cas_test}} was failing because calls to UnfilteredRowIterators.noRowsIterator
discard the static Columns data, which causes problems deserializing the static row.

Right, but the fix for that was sneaked into CASSANDRA-10045. Currently, if you look at {{UnfilteredRowIterators.noRowIterator}}
in your branch, it defines twice {{columns}}.

bq. The call to noRowsIterator from SelectNoSlices.makeSliceIterator was also discarding the
static row entirely.

That change makes sense however.

bq. \[...\] are also caused by CASSANDRA-9857

I've pushed a patch to that ticket. If you could validate that it does fix those tests for
you, that would be awesome.

bq. I've added a 'maybeDisableFetchAll' method to the column filter builder, which is called
from the legacy slice command deserializer, and turns off isFetchAll if the command isn't
selecting all regular/static columns.

I think what you want here is just to use {{ColumnFilter.selectionBuilder()}} to create the
builder in {{ReadCommand.LegacyReadCommandSerializer.SinglePartitionSliceCommand}}. In fact,
I'd say it's a bug that the ctor of {{ColumnFilter.Builder}} is public: it should be private
and one should always use one of the 2 static method {{selectionBuilder}} or {{allColumnsBuilder()}}
(so if you could also switch the ctor to private visibility in your patch, that would be great).

> Fix upgrade tests from #9704 that are still failing
> ---------------------------------------------------
>                 Key: CASSANDRA-9893
>                 URL:
>             Project: Cassandra
>          Issue Type: Bug
>            Reporter: Sylvain Lebresne
>            Assignee: Blake Eggleston
>             Fix For: 3.0 beta 2
> The first things to do on this ticket would be to commit Tyler's branch (
to the dtests so cassci run them. I've had to do a few minor modifications to have them run
locally so someone which access to cassci should do it and make sure it runs properly.
> Once we have that, we should fix any test that isn't passing. I've ran the tests locally
and I had 8 failures. for 2 of them, it sounds plausible that they'll get fixed by the patch
of CASSANDRA-9775, though that's just a guess.  The rest where test that timeouted without
a particular error in the log, and running some of them individually, they passed.  So we'll
have to see if it's just my machine being overly slow when running them all.

This message was sent by Atlassian JIRA

View raw message