You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "siying (via GitHub)" <gi...@apache.org> on 2024/05/21 22:56:54 UTC
[PR] [SPARK-48383] In KafkaOffsetReader, partition mismatch should not be an assertion [spark]
siying opened a new pull request, #46692:
URL: https://github.com/apache/spark/pull/46692
### What changes were proposed in this pull request?
Create a new error class START_OFFSET_DOES_NOT_MATCH_ASSIGNED. When partition mismatch is found in KafkaOffsetReader between start offsets and assigned partitions, throw this exception.
### Why are the changes needed?
In KafkaOffsetReader, we assert startOffsets have the same topic partition list as assigned. However, if the user changes topic partition while the query is running, they will see the assertion. Instead, they should see an exception.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Add a new unit test in KafkaOffsetReaderSuite to make sure the exception case is thrown correctly.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48383][SS] Throw better error for mismatched partitions in startOffset option in Kafka [spark]
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR commented on PR #46692:
URL: https://github.com/apache/spark/pull/46692#issuecomment-2144226643
I'd leave the audit of error class as post review. I'm merging this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48383][SS] In KafkaOffsetReader, partition mismatch should not be an assertion [spark]
Posted by "siying (via GitHub)" <gi...@apache.org>.
siying commented on PR #46692:
URL: https://github.com/apache/spark/pull/46692#issuecomment-2135757865
Hi, @MaxGekk can you take a look? I've addressed all the review comments from @HeartSaVioR .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48383][SS] Throw better error for mismatched partitions in startOffset option in Kafka [spark]
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR commented on PR #46692:
URL: https://github.com/apache/spark/pull/46692#issuecomment-2144226712
Thanks! Merging to master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48383] In KafkaOffsetReader, partition mismatch should not be an assertion [spark]
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR commented on code in PR #46692:
URL: https://github.com/apache/spark/pull/46692#discussion_r1609159915
##########
connector/kafka-0-10-sql/src/main/resources/error/kafka-error-conditions.json:
##########
@@ -23,6 +23,13 @@
"latest offset: <latestOffset>, end offset: <endOffset>"
]
},
+ "KAFKA_START_OFFSET_DOES_NOT_MATCH_ASSIGNED" : {
+ "message" : [
+ "Partitions specified for Kafka start offsets don't match what are assigned. Maybe topic partitions are created ",
+ "or deleted while the query is running. Use -1 for latest, -2 for earliest.",
+ "Specified: ${specifiedPartitions} Assigned: ${assignedPartitions}"
Review Comment:
We use `<>` for params in error message.
##########
connector/kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaOffsetReaderSuite.scala:
##########
@@ -135,6 +135,25 @@ class KafkaOffsetReaderSuite extends QueryTest with SharedSparkSession with Kafk
KafkaOffsetRange(tp, 2, LATEST, None)).sortBy(_.topicPartition.toString))
}
+ testWithAllOffsetFetchingSQLConf(
+ "SPARK-48383: START_OFFSET_DOES_NOT_MATCH_ASSIGNED error class"
+ ) {
+ val topic = newTopic()
+ testUtils.createTopic(topic, partitions = 3)
+ val reader = createKafkaReader(topic, minPartitions = Some(4))
+
+ // There are three topic partitions, but we only include two in offsets.
+ val tp1 = new TopicPartition(topic, 0)
+ val tp2 = new TopicPartition(topic, 1)
+ val startingOffsets = SpecificOffsetRangeLimit(Map(tp1 -> EARLIEST, tp2 -> EARLIEST))
+ val endingOffsets = SpecificOffsetRangeLimit(Map(tp1 -> LATEST, tp2 -> 3))
+
+ val ec = intercept[KafkaIllegalStateException] {
Review Comment:
Shall we use checkError to verify the error class related exception? Please let me know if it's not available for Kafka exceptions having error class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-48383][SS] Throw better error for mismatched partitions in startOffset option in Kafka [spark]
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR closed pull request #46692: [SPARK-48383][SS] Throw better error for mismatched partitions in startOffset option in Kafka
URL: https://github.com/apache/spark/pull/46692
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org