Skip to content

Commit 82138e0

Browse files
brkyvzzsxwing
authored andcommitted
[SPARK-19886] Fix reportDataLoss if statement in SS KafkaSource
## What changes were proposed in this pull request? Fix the `throw new IllegalStateException` if statement part. ## How is this patch tested Regression test Author: Burak Yavuz <brkyvz@gmail.com> Closes #17228 from brkyvz/kafka-cause-fix.
1 parent f79371a commit 82138e0

File tree

2 files changed

+54
-13
lines changed

2 files changed

+54
-13
lines changed

external/kafka-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/CachedKafkaConsumer.scala

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -273,19 +273,7 @@ private[kafka010] case class CachedKafkaConsumer private(
273273
message: String,
274274
cause: Throwable = null): Unit = {
275275
val finalMessage = s"$message ${additionalMessage(failOnDataLoss)}"
276-
if (failOnDataLoss) {
277-
if (cause != null) {
278-
throw new IllegalStateException(finalMessage)
279-
} else {
280-
throw new IllegalStateException(finalMessage, cause)
281-
}
282-
} else {
283-
if (cause != null) {
284-
logWarning(finalMessage)
285-
} else {
286-
logWarning(finalMessage, cause)
287-
}
288-
}
276+
reportDataLoss0(failOnDataLoss, finalMessage, cause)
289277
}
290278

291279
private def close(): Unit = consumer.close()
@@ -398,4 +386,23 @@ private[kafka010] object CachedKafkaConsumer extends Logging {
398386
consumer
399387
}
400388
}
389+
390+
private def reportDataLoss0(
391+
failOnDataLoss: Boolean,
392+
finalMessage: String,
393+
cause: Throwable = null): Unit = {
394+
if (failOnDataLoss) {
395+
if (cause != null) {
396+
throw new IllegalStateException(finalMessage, cause)
397+
} else {
398+
throw new IllegalStateException(finalMessage)
399+
}
400+
} else {
401+
if (cause != null) {
402+
logWarning(finalMessage, cause)
403+
} else {
404+
logWarning(finalMessage)
405+
}
406+
}
407+
}
401408
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.spark.sql.kafka010
19+
20+
import org.scalatest.PrivateMethodTester
21+
22+
import org.apache.spark.sql.test.SharedSQLContext
23+
24+
class CachedKafkaConsumerSuite extends SharedSQLContext with PrivateMethodTester {
25+
26+
test("SPARK-19886: Report error cause correctly in reportDataLoss") {
27+
val cause = new Exception("D'oh!")
28+
val reportDataLoss = PrivateMethod[Unit]('reportDataLoss0)
29+
val e = intercept[IllegalStateException] {
30+
CachedKafkaConsumer.invokePrivate(reportDataLoss(true, "message", cause))
31+
}
32+
assert(e.getCause === cause)
33+
}
34+
}

0 commit comments

Comments
 (0)