On the patch itself, and as far as removing clocks from thrift api is
concerned, few minor comments:
- In test_thrift_server.py, there is a Clock remaining in test_cf_recreate().
I also get 3 errors in the avro system tests, but it seems unrelated.
- As remarked by Johan, in the thift interface, the timestamp is still called
clock. It's not a bad name per se, but renaming it back to timestamp would
ease migration for those coming from 0.6.
- Cli client doesn't remove fully reconciler and clock_type (in the
AddColumnFamilyArgument enum and at line 261, in the add column family
documentation). It's not completely necessary, but as is the
assert at the end of the switch in executeAddColumnFamily() is wrong.
The patch does not remove the clocks from all of what I could call 'the
cassandra exported api'. I'm mostly thinking of:
- the avro API.
- the fact that DatabaseDescriptor.readTablesFromYaml() (and
RawColumnFamily) still tries to read a reconciler and clock_type.
The ticket's name makes it clear that it is about thrift, and if not done
before, that will have to be tackled in #1502, but as it's not strictly
speaking 'internals', I thought I'd mentionned it (in particular, whatever we
CASSANDRA-1502, those will have to be done).
I agree that for
CASSANDRA-1072, we need some way to distinguish between cfs with
counters and other cfs. But if we're just talking counters, and as far as the
thrift api is concerned, it will be enough to have a 'Counter' cf type (and
a 'SuperCounter' one). Or, alternatively, we could user the validateWith
option to distinguish counter cfs. So I see no reason why we shouldn't remove
clock_type and reconciler in this patch. Modulo the (minor) comments above,
I'm +1 on this.
Removing the clock interface from the internals is another matter, but I'll
comment on the