You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Aleksandr Sorokoumov (Jira)" <ji...@apache.org> on 2021/07/17 16:13:00 UTC
[jira] [Comment Edited] (CASSANDRA-16349) SSTableLoader reports
error when SSTable(s) do not have data for some nodes
[ https://issues.apache.org/jira/browse/CASSANDRA-16349?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17382609#comment-17382609 ]
Aleksandr Sorokoumov edited comment on CASSANDRA-16349 at 7/17/21, 4:12 PM:
----------------------------------------------------------------------------
*Short version of the review*
* The bug is reproducible in 4.0+
* The fix for SSTableLoader LGTM as a way to avoid useless streaming tasks
* I added a python dtest for the issue
* We should also fix the way streaming handles empty SSTables after CASSANDRA-14115
*Code and CI*
||branch||CI||
|[dtest|https://github.com/apache/cassandra-dtest/pull/151]| |
|[4.0 (baseline)|https://github.com/gerrrr/cassandra/tree/cassandra-4.0-16349-dtest]|[j8|https://app.circleci.com/pipelines/github/Gerrrr/cassandra/188/workflows/27d68d7c-3ae8-4dcd-869b-d8bbd47157a4] [j11|https://app.circleci.com/pipelines/github/Gerrrr/cassandra/188/workflows/844e33c6-1327-439d-980f-0112cf958829]|
|[SSTableLoader fix|https://github.com/apache/cassandra/compare/trunk...Gerrrr:16349-sstableloader-fix-4.0?expand=1]|[j8|https://app.circleci.com/pipelines/github/Gerrrr/cassandra/189/workflows/2a308aa6-6ff6-4294-842a-6e691831c59f] [j11|https://app.circleci.com/pipelines/github/Gerrrr/cassandra/189/workflows/c9729460-f035-49ab-873d-14f0cf6e2cc5]|
|[Streaming fix|https://github.com/apache/cassandra/compare/trunk...Gerrrr:16349-streaming-fix-4.0?expand=1]|[j8|https://app.circleci.com/pipelines/github/Gerrrr/cassandra/191/workflows/4855a5e0-8ba3-4007-8e87-3c4094702b53] [j11|https://app.circleci.com/pipelines/github/Gerrrr/cassandra/191/workflows/b3e1df91-fcae-410f-a3d7-2f5176203586]|
|[Streaming fix + SSTableLoader fix|https://github.com/apache/cassandra/compare/trunk...Gerrrr:16349-streaming-sstableloader-4.0?expand=1]|[j8|https://app.circleci.com/pipelines/github/Gerrrr/cassandra/192/workflows/0918fc51-7492-467b-8f87-8ea46830f262] [j11|https://app.circleci.com/pipelines/github/Gerrrr/cassandra/192/workflows/0d15992d-c33b-4955-a531-e8c371beab15]|
*Long version of the review*
I was able to reproduce the bug following the steps in the issue description in {{cassandra-4.0}} and {{trunk}}. The issue does not reproduce in the earlier versions. Given no changes in the SSTableLoader between {{3.11}} and {{trunk}}, it got me curious if the fix should be on the streaming side instead.
AFAIU the failing assertion ([link|https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/db/streaming/CassandraIncomingFile.java#L96]) was introduced in CASSANDRA-14115 as a sanity check that the file's size is not accessed before it has been read. However, this assertion might be incorrect as the default state for the size is -1, and the intention is to verify that the value has been updated.
As an experiment, I changed the assertion in {{getSize}} and re-ran the test. Streaming tasks started to crash in [StreamReceiveTask#receive|https://github.com/apache/cassandra/blob/9cc7a0025d8b0859d8e9c947f6fdffd8455dd141/src/java/org/apache/cassandra/streaming/StreamReceiveTask.java#L87] to due [no open SSTable writers|https://github.com/apache/cassandra/blob/9cc7a0025d8b0859d8e9c947f6fdffd8455dd141/src/java/org/apache/cassandra/io/sstable/format/RangeAwareSSTableWriter.java#L168-L171].
In my opinion, this is a bug as C* could handle streaming empty SSTables in prior versions, so I created a patch that handles empty streams without throwing exceptions. Even though it works without Serban's SSTableLoader fix, we should include it to prevent SSTableLoader from doing unnecessary work.
was (Author: gerrrr):
*The short version of the review*
* The bug is reproducible in 4.0+
* The fix for SSTableLoader LGTM as a way to avoid useless streaming tasks
* I added a python dtest for the issue
* We should also fix the way streaming handles empty SSTables after CASSANDRA-14115
*Code and CI*
||branch||CI|
|[dtest|https://github.com/apache/cassandra-dtest/pull/151]| |
|[4.0 (baseline)|https://github.com/gerrrr/cassandra/tree/cassandra-4.0-16349-dtest]|[j8|https://app.circleci.com/pipelines/github/Gerrrr/cassandra/188/workflows/27d68d7c-3ae8-4dcd-869b-d8bbd47157a4] [j11|https://app.circleci.com/pipelines/github/Gerrrr/cassandra/188/workflows/844e33c6-1327-439d-980f-0112cf958829]
|[SSTableLoader fix|https://github.com/apache/cassandra/compare/trunk...Gerrrr:16349-sstableloader-fix-4.0?expand=1]|[j8|https://app.circleci.com/pipelines/github/Gerrrr/cassandra/189/workflows/2a308aa6-6ff6-4294-842a-6e691831c59f] [j11|https://app.circleci.com/pipelines/github/Gerrrr/cassandra/189/workflows/c9729460-f035-49ab-873d-14f0cf6e2cc5]
|[Streaming fix|https://github.com/apache/cassandra/compare/trunk...Gerrrr:16349-streaming-fix-4.0?expand=1]|[j8|https://app.circleci.com/pipelines/github/Gerrrr/cassandra/191/workflows/4855a5e0-8ba3-4007-8e87-3c4094702b53] [j11|https://app.circleci.com/pipelines/github/Gerrrr/cassandra/191/workflows/b3e1df91-fcae-410f-a3d7-2f5176203586]
|[Streaming fix + SSTableLoader fix|https://github.com/apache/cassandra/compare/trunk...Gerrrr:16349-streaming-sstableloader-4.0?expand=1]|[j8|https://app.circleci.com/pipelines/github/Gerrrr/cassandra/192/workflows/0918fc51-7492-467b-8f87-8ea46830f262] [j11|https://app.circleci.com/pipelines/github/Gerrrr/cassandra/192/workflows/0d15992d-c33b-4955-a531-e8c371beab15]
*Long version of the review*
I was able to reproduce the bug following the steps in the issue description in {{cassandra-4.0}} and {{trunk}}. The issue does not reproduce in the earlier versions. Given no changes in the SSTableLoader between {{3.11}} and {{trunk}}, it got me curious if the fix should be on the streaming side instead.
AFAIU the failing assertion ([link|https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/db/streaming/CassandraIncomingFile.java#L96]) was introduced in CASSANDRA-14115 as a sanity check that the file's size is not accessed before it has been read. However, this assertion might be incorrect as the default state for the size is -1, and the intention is to verify that the value has been updated.
As an experiment, I changed the assertion in {{getSize}} and re-ran the test. Streaming tasks started to crash in [StreamReceiveTask#receive|https://github.com/apache/cassandra/blob/9cc7a0025d8b0859d8e9c947f6fdffd8455dd141/src/java/org/apache/cassandra/streaming/StreamReceiveTask.java#L87] to due [no open SSTable writers|https://github.com/apache/cassandra/blob/9cc7a0025d8b0859d8e9c947f6fdffd8455dd141/src/java/org/apache/cassandra/io/sstable/format/RangeAwareSSTableWriter.java#L168-L171].
In my opinion, this is a bug as C* could handle streaming empty SSTables in prior versions, so I created a patch that handles empty streams without throwing exceptions. Even though it works without Serban's SSTableLoader fix, we should include it to prevent SSTableLoader from doing unnecessary work.
> SSTableLoader reports error when SSTable(s) do not have data for some nodes
> ---------------------------------------------------------------------------
>
> Key: CASSANDRA-16349
> URL: https://issues.apache.org/jira/browse/CASSANDRA-16349
> Project: Cassandra
> Issue Type: Bug
> Components: Tool/sstable
> Reporter: Serban Teodorescu
> Assignee: Serban Teodorescu
> Priority: Normal
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Running SSTableLoader in verbose mode will show error(s) if there are node(s) that do not own any data from the SSTable(s). This can happen in at least 2 cases:
> # SSTableLoader is used to stream backups while keeping the same token ranges
> # SSTable(s) are created with CQLSSTableWriter to match token ranges (this can bring better performance by using ZeroCopy streaming)
> Partial output of the SSTableLoader:
> {quote}ERROR 02:47:47,842 [Stream #fa8e73b0-3da5-11eb-9c47-c5d27ae8fe47] Remote peer /127.0.0.4:7000 failed stream session.
> ERROR 02:47:47,842 [Stream #fa8e73b0-3da5-11eb-9c47-c5d27ae8fe47] Remote peer /127.0.0.3:7000 failed stream session.
> progress: [/127.0.0.4:7000]0:0/1 100% [/127.0.0.3:7000]0:0/1 100% [/127.0.0.2:7000]0:7/7 100% [/127.0.0.1:7000]0:7/7 100% total: 100% 0.000KiB/s (avg: 1.611KiB/s)
> progress: [/127.0.0.4:7000]0:0/1 100% [/127.0.0.3:7000]0:0/1 100% [/127.0.0.2:7000]0:7/7 100% [/127.0.0.1:7000]0:7/7 100% total: 100% 0.000KiB/s (avg: 1.611KiB/s)
> progress: [/127.0.0.4:7000]0:0/1 100% [/127.0.0.3:7000]0:0/1 100% [/127.0.0.2:7000]0:7/7 100% [/127.0.0.1:7000]0:7/7 100% total: 100% 0.000KiB/s (avg: 1.515KiB/s)
> progress: [/127.0.0.4:7000]0:0/1 100% [/127.0.0.3:7000]0:0/1 100% [/127.0.0.2:7000]0:7/7 100% [/127.0.0.1:7000]0:7/7 100% total: 100% 0.000KiB/s (avg: 1.427KiB/s)
> {quote}
>
> Stack trace:
> {quote}java.util.concurrent.ExecutionException: org.apache.cassandra.streaming.StreamException: Stream failed
> at com.google.common.util.concurrent.AbstractFuture.getDoneValue(AbstractFuture.java:552)
> at com.google.common.util.concurrent.AbstractFuture.get(AbstractFuture.java:533)
> at org.apache.cassandra.tools.BulkLoader.load(BulkLoader.java:99)
> at org.apache.cassandra.tools.BulkLoader.main(BulkLoader.java:49)
> Caused by: org.apache.cassandra.streaming.StreamException: Stream failed
> at org.apache.cassandra.streaming.management.StreamEventJMXNotifier.onFailure(StreamEventJMXNotifier.java:88)
> at com.google.common.util.concurrent.Futures$CallbackListener.run(Futures.java:1056)
> at com.google.common.util.concurrent.DirectExecutor.execute(DirectExecutor.java:30)
> at com.google.common.util.concurrent.AbstractFuture.executeListener(AbstractFuture.java:1138)
> at com.google.common.util.concurrent.AbstractFuture.complete(AbstractFuture.java:958)
> at com.google.common.util.concurrent.AbstractFuture.setException(AbstractFuture.java:748)
> at org.apache.cassandra.streaming.StreamResultFuture.maybeComplete(StreamResultFuture.java:220)
> at org.apache.cassandra.streaming.StreamResultFuture.handleSessionComplete(StreamResultFuture.java:196)
> at org.apache.cassandra.streaming.StreamSession.closeSession(StreamSession.java:505)
> at org.apache.cassandra.streaming.StreamSession.complete(StreamSession.java:819)
> at org.apache.cassandra.streaming.StreamSession.messageReceived(StreamSession.java:595)
> at org.apache.cassandra.streaming.async.StreamingInboundHandler$StreamDeserializingTask.run(StreamingInboundHandler.java:189)
> at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
> at java.base/java.lang.Thread.run(Thread.java:844)
> {quote}
> To reproduce create a cluster with ccm with more nodes than the RF, put some data into it copy a SSTable and stream it.
>
> The error originates on the nodes, the following stack trace is shown in the logs:
> {quote}java.lang.IllegalStateException: Stream hasn't been read yet
> at com.google.common.base.Preconditions.checkState(Preconditions.java:507)
> at org.apache.cassandra.db.streaming.CassandraIncomingFile.getSize(CassandraIncomingFile.java:96)
> at org.apache.cassandra.streaming.StreamSession.receive(StreamSession.java:789)
> at org.apache.cassandra.streaming.StreamSession.messageReceived(StreamSession.java:587)
> at org.apache.cassandra.streaming.async.StreamingInboundHandler$StreamDeserializingTask.run(StreamingInboundHandler.java:189)
> at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
> at java.base/java.lang.Thread.run(Thread.java:844)
> {quote}
>
> An error is thrown due to stream size being read before any data was received. The solution would be not to stream this at all; SSTableLoader.java already looks into each SSTable to determine what parts of it will map to each node token ranges.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org