[jira] [Commented] (CASSANDRA-9949) maxPurgeableTimestamp needs to check memtables too
[ https://issues.apache.org/jira/browse/CASSANDRA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15114791#comment-15114791 ] Stefania commented on CASSANDRA-9949: - CI looks good, ready for review [~krummas]. > maxPurgeableTimestamp needs to check memtables too > -- > > Key: CASSANDRA-9949 > URL: https://issues.apache.org/jira/browse/CASSANDRA-9949 > Project: Cassandra > Issue Type: Bug > Components: Local Write-Read Paths >Reporter: Jonathan Ellis >Assignee: Stefania > Fix For: 2.2.x, 3.0.x, 3.x > > > overlapIterator/maxPurgeableTimestamp don't include the memtables, so a > very-out-of-order write could be ignored -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-9949) maxPurgeableTimestamp needs to check memtables too
[
https://issues.apache.org/jira/browse/CASSANDRA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15112131#comment-15112131
]
Stefania commented on CASSANDRA-9949:
-
The 3.0 patch is ready as well. This patch is much simpler than the 2.2 version
because each partition already has the minimum timestamp in
{{stats().minTimestamp}}. There is a small conflict from 3.0 to 3.3 (very easy
to resolve) and then it merges clearly from 3.3 to trunk.
I'm still monitoring CI and I will move the ticket to patch available once CI
is confirmed fine.
||3.0||3.3||trunk||
|[patch|https://github.com/stef1927/cassandra/commits/9949-3.0]|[patch|https://github.com/stef1927/cassandra/commits/9949-3.3]|-|
|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-9949-3.0-testall/]|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-9949-3.3-testall/]|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-9949-testall/]|
|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-9949-3.0-dtest/]|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-9949-3.3-dtest/]|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-9949-dtest/]|
> maxPurgeableTimestamp needs to check memtables too
> --
>
> Key: CASSANDRA-9949
> URL: https://issues.apache.org/jira/browse/CASSANDRA-9949
> Project: Cassandra
> Issue Type: Bug
> Components: Local Write-Read Paths
>Reporter: Jonathan Ellis
>Assignee: Stefania
> Fix For: 2.2.x, 3.0.x, 3.x
>
>
> overlapIterator/maxPurgeableTimestamp don't include the memtables, so a
> very-out-of-order write could be ignored
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-9949) maxPurgeableTimestamp needs to check memtables too
[ https://issues.apache.org/jira/browse/CASSANDRA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15107722#comment-15107722 ] Stefania commented on CASSANDRA-9949: - Thanks, starting with the 3.0+ patch then. > maxPurgeableTimestamp needs to check memtables too > -- > > Key: CASSANDRA-9949 > URL: https://issues.apache.org/jira/browse/CASSANDRA-9949 > Project: Cassandra > Issue Type: Bug > Components: Local Write-Read Paths >Reporter: Jonathan Ellis >Assignee: Stefania > Fix For: 2.2.x, 3.0.x, 3.x > > > overlapIterator/maxPurgeableTimestamp don't include the memtables, so a > very-out-of-order write could be ignored -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-9949) maxPurgeableTimestamp needs to check memtables too
[ https://issues.apache.org/jira/browse/CASSANDRA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15106541#comment-15106541 ] Marcus Eriksson commented on CASSANDRA-9949: I doubt it matters much which timestamp we pick there - this should happen very rarely and when it does, there is not much harm (we keep some tombstones around for a bit longer) so lets keep it consistent with the apply(Cell insert) functionality The 2.2-patch LGTM > maxPurgeableTimestamp needs to check memtables too > -- > > Key: CASSANDRA-9949 > URL: https://issues.apache.org/jira/browse/CASSANDRA-9949 > Project: Cassandra > Issue Type: Bug > Components: Local Write-Read Paths >Reporter: Jonathan Ellis >Assignee: Stefania > Fix For: 2.2.x, 3.0.x, 3.x > > > overlapIterator/maxPurgeableTimestamp don't include the memtables, so a > very-out-of-order write could be ignored -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-9949) maxPurgeableTimestamp needs to check memtables too
[
https://issues.apache.org/jira/browse/CASSANDRA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15106398#comment-15106398
]
Stefania commented on CASSANDRA-9949:
-
Thank you for your review [~krummas], please see [this commit|
https://github.com/stef1927/cassandra/commit/96dacb1c2f555d34753901e48fecf1f8f0190393].
I've clarified the comment best I could and added a test to show what I mean.
If it still doesn't make much sense to you, perhaps we should just change the
test and pick the reconciled timestamp. I must admit I am not 100% sure of
what's best.
I've updated {{CompactionController.getFullyExpiredSSTables()}} and added a
unit test for it.
> maxPurgeableTimestamp needs to check memtables too
> --
>
> Key: CASSANDRA-9949
> URL: https://issues.apache.org/jira/browse/CASSANDRA-9949
> Project: Cassandra
> Issue Type: Bug
> Components: Local Write-Read Paths
>Reporter: Jonathan Ellis
>Assignee: Stefania
> Fix For: 2.2.x, 3.0.x, 3.x
>
>
> overlapIterator/maxPurgeableTimestamp don't include the memtables, so a
> very-out-of-order write could be ignored
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-9949) maxPurgeableTimestamp needs to check memtables too
[ https://issues.apache.org/jira/browse/CASSANDRA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15105471#comment-15105471 ] Marcus Eriksson commented on CASSANDRA-9949: The approach looks good to me, two comments; * could you rephrase [this comment|https://github.com/stef1927/cassandra/blob/aabec6bab0d4558b7ce9bfbdec17909ff0b90bc9/src/java/org/apache/cassandra/db/AtomicBTreeColumns.java#L474-L476]? I don't get why we can't use the reconciled version. * We should probably check the memtable min timestamp in CompactionController.getFullyExpiredSSTables as well > maxPurgeableTimestamp needs to check memtables too > -- > > Key: CASSANDRA-9949 > URL: https://issues.apache.org/jira/browse/CASSANDRA-9949 > Project: Cassandra > Issue Type: Bug > Components: Local Write-Read Paths >Reporter: Jonathan Ellis >Assignee: Stefania > Fix For: 2.2.x, 3.0.x, 3.x > > > overlapIterator/maxPurgeableTimestamp don't include the memtables, so a > very-out-of-order write could be ignored -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-9949) maxPurgeableTimestamp needs to check memtables too
[ https://issues.apache.org/jira/browse/CASSANDRA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15096299#comment-15096299 ] Stefania commented on CASSANDRA-9949: - Thanks. > maxPurgeableTimestamp needs to check memtables too > -- > > Key: CASSANDRA-9949 > URL: https://issues.apache.org/jira/browse/CASSANDRA-9949 > Project: Cassandra > Issue Type: Bug > Components: Local Write-Read Paths >Reporter: Jonathan Ellis >Assignee: Stefania > Fix For: 2.2.x, 3.0.x, 3.x > > > overlapIterator/maxPurgeableTimestamp don't include the memtables, so a > very-out-of-order write could be ignored -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-9949) maxPurgeableTimestamp needs to check memtables too
[ https://issues.apache.org/jira/browse/CASSANDRA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15096294#comment-15096294 ] Philip Thompson commented on CASSANDRA-9949: It looks like the second run is working now. I'll try to add better debug logging so we know why the original happened. > maxPurgeableTimestamp needs to check memtables too > -- > > Key: CASSANDRA-9949 > URL: https://issues.apache.org/jira/browse/CASSANDRA-9949 > Project: Cassandra > Issue Type: Bug > Components: Local Write-Read Paths >Reporter: Jonathan Ellis >Assignee: Stefania > Fix For: 2.2.x, 3.0.x, 3.x > > > overlapIterator/maxPurgeableTimestamp don't include the memtables, so a > very-out-of-order write could be ignored -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-9949) maxPurgeableTimestamp needs to check memtables too
[
https://issues.apache.org/jira/browse/CASSANDRA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15096194#comment-15096194
]
Stefania commented on CASSANDRA-9949:
-
There is something really weird with the [first dtest
run|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-9949-2.2-dtest/lastSuccessfulBuild/]:
all tests are failing with {{ERROR: Cassandra 3.0+ requires Java >= 1.8, found
Java 1.7}} but this is a 2.2 branch as confirmed by the version in _build.xml_.
cc [~philipthompson]
Relaunched.
> maxPurgeableTimestamp needs to check memtables too
> --
>
> Key: CASSANDRA-9949
> URL: https://issues.apache.org/jira/browse/CASSANDRA-9949
> Project: Cassandra
> Issue Type: Bug
> Components: Local Write-Read Paths
>Reporter: Jonathan Ellis
>Assignee: Stefania
> Fix For: 2.1.x, 2.2.x
>
>
> overlapIterator/maxPurgeableTimestamp don't include the memtables, so a
> very-out-of-order write could be ignored
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-9949) maxPurgeableTimestamp needs to check memtables too
[
https://issues.apache.org/jira/browse/CASSANDRA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15096157#comment-15096157
]
Stefania commented on CASSANDRA-9949:
-
The patch for 2.2 is ready, I would like a review before I implement the
equivalent for 3.0+.
I decided to track the min timestamp at the memtable level, when a new
partition is inserted. There is an inexpensive way to calculate the minimum
timestamp by piggy-backing on the {{BTree}} update loop on cells via
{{AtomicBTreeColumns.ColumnUpdater}}, specifically its {{apply}} methods. The
downside is that if a cell is reconciled with another cell with a later
timestamp, we pick the smallest of the two timestamps even though technically
the latest timestamp should be sufficient. This shouldn't matter though, since
we then pick the smallest timestamp across all partitions in the memtable. I
hope this is a good enough approximation.
I've also added a unit test which should be sufficient to test the new
functionality.
||2.2||
|[patch|https://github.com/stef1927/cassandra/commits/9949-2.2]|
|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-9949-2.2-testall/]|
|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-9949-2.2-dtest/]|
CI is still running.
> maxPurgeableTimestamp needs to check memtables too
> --
>
> Key: CASSANDRA-9949
> URL: https://issues.apache.org/jira/browse/CASSANDRA-9949
> Project: Cassandra
> Issue Type: Bug
> Components: Local Write-Read Paths
>Reporter: Jonathan Ellis
>Assignee: Stefania
> Fix For: 2.1.x, 2.2.x
>
>
> overlapIterator/maxPurgeableTimestamp don't include the memtables, so a
> very-out-of-order write could be ignored
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-9949) maxPurgeableTimestamp needs to check memtables too
[ https://issues.apache.org/jira/browse/CASSANDRA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15092139#comment-15092139 ] Stefania commented on CASSANDRA-9949: - Thanks for clarifying the fix version. Re. performance are you concerned by the loop on all cells in a column family? We could approximate the min timestamp with the memtable creation time but I don't think this is a good approximation and it's probably wrong if we receive a partition from a remote host with an older timestamp. Storing the minimum timestamp in a column family and keeping it up-to-date requires a bit of work but we can go down this route if it is the correct choice. However I feel there may be a different approximation that I am missing. I also could use some hints on how to test this. Do you think a dtest is achievable? I'm not so sure how easy it is to simulate a 'very-out-of-order' write. > maxPurgeableTimestamp needs to check memtables too > -- > > Key: CASSANDRA-9949 > URL: https://issues.apache.org/jira/browse/CASSANDRA-9949 > Project: Cassandra > Issue Type: Bug > Components: Local Write-Read Paths >Reporter: Jonathan Ellis >Assignee: Stefania > Fix For: 2.1.x, 2.2.x > > > overlapIterator/maxPurgeableTimestamp don't include the memtables, so a > very-out-of-order write could be ignored -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-9949) maxPurgeableTimestamp needs to check memtables too
[ https://issues.apache.org/jira/browse/CASSANDRA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15086350#comment-15086350 ] Aleksey Yeschenko commented on CASSANDRA-9949: -- I also wonder what the effect on performance is when implemented this way > maxPurgeableTimestamp needs to check memtables too > -- > > Key: CASSANDRA-9949 > URL: https://issues.apache.org/jira/browse/CASSANDRA-9949 > Project: Cassandra > Issue Type: Bug > Components: Local Write-Read Paths >Reporter: Jonathan Ellis >Assignee: Stefania > Fix For: 2.1.x, 2.2.x > > > overlapIterator/maxPurgeableTimestamp don't include the memtables, so a > very-out-of-order write could be ignored -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-9949) maxPurgeableTimestamp needs to check memtables too
[ https://issues.apache.org/jira/browse/CASSANDRA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15086347#comment-15086347 ] Aleksey Yeschenko commented on CASSANDRA-9949: -- [~Stefania] I would stick to 2.2.x+ > maxPurgeableTimestamp needs to check memtables too > -- > > Key: CASSANDRA-9949 > URL: https://issues.apache.org/jira/browse/CASSANDRA-9949 > Project: Cassandra > Issue Type: Bug > Components: Local Write-Read Paths >Reporter: Jonathan Ellis >Assignee: Stefania > Fix For: 2.1.x, 2.2.x > > > overlapIterator/maxPurgeableTimestamp don't include the memtables, so a > very-out-of-order write could be ignored -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-9949) maxPurgeableTimestamp needs to check memtables too
[ https://issues.apache.org/jira/browse/CASSANDRA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15081334#comment-15081334 ] Stefania commented on CASSANDRA-9949: - [~iamaleksey] : can you confirm this is required on all versions, including 2.1? Is looking at memtables when calculating the max purgeable timestamp all that's required (along with swapping the predicate and tests)? See [here|https://github.com/stef1927/cassandra/commit/e5be5cfafcfd203e1267367d94ae38c03d9d3793] for 2.1. > maxPurgeableTimestamp needs to check memtables too > -- > > Key: CASSANDRA-9949 > URL: https://issues.apache.org/jira/browse/CASSANDRA-9949 > Project: Cassandra > Issue Type: Bug > Components: Local Write-Read Paths >Reporter: Jonathan Ellis >Assignee: Stefania > Fix For: 2.1.x, 2.2.x > > > overlapIterator/maxPurgeableTimestamp don't include the memtables, so a > very-out-of-order write could be ignored -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-9949) maxPurgeableTimestamp needs to check memtables too
[ https://issues.apache.org/jira/browse/CASSANDRA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14649971#comment-14649971 ] Aleksey Yeschenko commented on CASSANDRA-9949: -- Very out of order, or just with ridiculously low gc gs. This issue was raised by [~benedict] during CASSANDRA-6230 review, originally. > maxPurgeableTimestamp needs to check memtables too > -- > > Key: CASSANDRA-9949 > URL: https://issues.apache.org/jira/browse/CASSANDRA-9949 > Project: Cassandra > Issue Type: Bug > Components: Core >Reporter: Jonathan Ellis >Assignee: Stefania > Fix For: 2.1.x, 2.2.x > > > overlapIterator/maxPurgeableTimestamp don't include the memtables, so a > very-out-of-order write could be ignored -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-9949) maxPurgeableTimestamp needs to check memtables too
[
https://issues.apache.org/jira/browse/CASSANDRA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14649931#comment-14649931
]
Jonathan Ellis commented on CASSANDRA-9949:
---
Nit: should probably reverse the order of the predicates in {{timestamp <
getMaxPurgeableTimestamp() && localDeletionTime < gcBefore}} since the former
is expensive while the latter is not.
> maxPurgeableTimestamp needs to check memtables too
> --
>
> Key: CASSANDRA-9949
> URL: https://issues.apache.org/jira/browse/CASSANDRA-9949
> Project: Cassandra
> Issue Type: Bug
> Components: Core
>Reporter: Jonathan Ellis
> Fix For: 2.1.x, 2.2.x
>
>
> overlapIterator/maxPurgeableTimestamp don't include the memtables, so a
> very-out-of-order write could be ignored
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
