You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Aleksey Yeschenko (JIRA)" <ji...@apache.org> on 2016/12/05 17:38:59 UTC

[jira] [Commented] (CASSANDRA-12982) Customizable hint ttl has been removed

    [ https://issues.apache.org/jira/browse/CASSANDRA-12982?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15722822#comment-15722822 ] 

Aleksey Yeschenko commented on CASSANDRA-12982:
-----------------------------------------------

I think the thought process back then was "we don't use cells with TTL to store hints anymore, so this makes no sense to re-add".

Except we kind of do have, in the form of gcgs vint. So I don't mind re-adding it.

The patch is no bad, but I feel like we can do better:

1. You've added code poking into the buffer containing the hint to {{HintsReader}}. Arguably, this code doesn't belong to {{HIntsReader}} - it's not supposed to be aware of the underlying {{Hint}} representation, only binary encoding of the log itself. That code should live in {{Hint.Serializer}}. Besides breaking separation of concerns this duplication adds risk of forgetting to update deser code in both classes when we change it.

2. It should be versioned.

3. If we properly move it to {{Hint.Serializer}}, we can take this further, and besides cutting some network I/O, also short-circuit hint/buffer reading after getting the first two fields. For verbatim buffer iterators it means we can skip allocating a {{ByteBuffer}} with the containing mutation. For {{Hint}} iterator this means we can avoid deserialising the whole mutation entirely if we can deduct from creation time, gcgs, and max ttl that the hint is no longer alive.

4. There really is no need to call {{System.currentTimeMillis()}} for every hint deserialisation (via {{isLive}} check). This can be pretty expensive in certain virtualised environments depending on the configured time source. We can calculate it once on each iterator init (once per page), and still never overstream (if this is a normal dispatch, the hints are all still young; if it's a late dispatch - after a node being down for a while - the hints will likely be either all alive or all dead).

Pushed a branch with suggestions [here|https://github.com/iamaleksey/cassandra/commits/12982-sketch]. Note: it has some unrelated code changes (which ended up as CASSANDRA-12998), also doesn't necessarily work, or pass tests. Coded up while bored on Sunday at night, so there might be some room for improvement, too.

Please let me know what you think.

> Customizable hint ttl has been removed
> --------------------------------------
>
>                 Key: CASSANDRA-12982
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-12982
>             Project: Cassandra
>          Issue Type: Bug
>            Reporter: Blake Eggleston
>            Assignee: Blake Eggleston
>            Priority: Minor
>             Fix For: 3.x
>
>
> The cassandra.maxHintTTL property added in CASSANDRA-5988 was removed in the hint system rewrite for 3.0.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)