Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
76913e2
Batch oriented kafka rdd, WIP. todo: cluster metadata / finding leader
koeninger Nov 23, 2014
1d70625
WIP on kafka cluster
koeninger Nov 23, 2014
0b94b33
use dropWhile rather than filter to trim beginning of fetch response
koeninger Nov 24, 2014
4dafd1b
method to get leader offsets, switch rdd bound to being exclusive sta…
koeninger Nov 24, 2014
ce91c59
method to get consumer offsets, explicit error handling
koeninger Nov 24, 2014
7d050bc
methods to set consumer offsets and get topic metadata, switch back t…
koeninger Nov 24, 2014
783b477
update tests for kafka 8.1.1
koeninger Nov 25, 2014
29c6b43
cleanup logging
koeninger Nov 25, 2014
3c2a96a
fix scalastyle errors
koeninger Nov 25, 2014
4b078bf
differentiate between leader and consumer offsets in error message
koeninger Nov 25, 2014
8d7de4a
make sure leader offsets can be found even for leaders that arent in …
koeninger Nov 25, 2014
979da25
dont allow empty leader offsets to be returned
koeninger Nov 26, 2014
38bb727
give easy access to the parameters of a KafkaRDD
koeninger Dec 3, 2014
326ff3c
add some tests
koeninger Dec 16, 2014
6bf14f2
first attempt at a Kafka dstream that allows for exactly-once semantics
koeninger Dec 24, 2014
bcca8a4
Merge branch 'master' of https://github.com/apache/spark into kafkaRdd
koeninger Dec 24, 2014
37d3053
make KafkaRDDPartition available to users so offsets can be committed…
koeninger Dec 25, 2014
cac63ee
additional testing, fix fencepost error
koeninger Dec 25, 2014
e09045b
[SPARK-4964] add foreachPartitionWithIndex, to avoid doing equivalent…
koeninger Dec 26, 2014
8bfd6c0
[SPARK-4964] configure rate limiting via spark.streaming.receiver.max…
koeninger Dec 30, 2014
1d50749
[SPARK-4964] code cleanup per tdas
koeninger Dec 30, 2014
adf99a6
[SPARK-4964] fix serialization issues for checkpointing
koeninger Jan 5, 2015
356c7cc
[SPARK-4964] code cleanup per helena
koeninger Jan 9, 2015
e93eb72
[SPARK-4964] refactor to add preferredLocations. depends on SPARK-4014
koeninger Jan 9, 2015
e86317b
[SPARK-4964] try seed brokers in random order to spread metadata requ…
koeninger Jan 10, 2015
0458e4e
[SPARK-4964] recovery of generated rdds from checkpoint
koeninger Jan 10, 2015
548d529
Merge branch 'master' of https://github.com/apache/spark into kafkaRdd
koeninger Jan 14, 2015
c1bd6d9
[SPARK-4964] use newly available attemptNumber for correct retry beha…
koeninger Jan 14, 2015
d4a7cf7
[SPARK-4964] allow for use cases that need to override compute for cu…
koeninger Jan 14, 2015
bb80bbe
[SPARK-4964] scalastyle line length
koeninger Jan 26, 2015
2e67117
[SPARK-4964] one potential way of hiding most of the implementation, …
koeninger Jan 28, 2015
19406cc
Merge branch 'master' of https://github.com/apache/spark into kafkaRdd
koeninger Jan 28, 2015
99d2eba
[SPARK-4964] Reduce level of nesting. If beginning is past end, its …
koeninger Jan 29, 2015
80fd6ae
[SPARK-4964] Rename createExactlyOnceStream so it isnt over-promising…
koeninger Jan 29, 2015
2b340d8
[SPARK-4964] refactor per TD feedback
koeninger Jan 30, 2015
9a838c2
[SPARK-4964] code cleanup, add more tests
koeninger Jan 30, 2015
0090553
[SPARK-4964] javafication of interfaces
koeninger Jan 30, 2015
9adaa0a
[SPARK-4964] formatting
koeninger Jan 30, 2015
4354bce
[SPARK-4964] per td, remove java interfaces, replace with final class…
koeninger Feb 3, 2015
825110f
[SPARK-4964] rename stuff per TD
koeninger Feb 3, 2015
8991017
[SPARK-4964] formatting
koeninger Feb 3, 2015
0df3ebe
[SPARK-4964] add comments per pwendell / dibbhatt
koeninger Feb 3, 2015
8c31855
[SPARK-4964] remove HasOffsetRanges interface from return types
koeninger Feb 4, 2015
59e29f6
[SPARK-4964] settle on "Direct" as a naming convention for the new st…
koeninger Feb 4, 2015
1dc2941
[SPARK-4964] silence ConsumerConfig warnings about broker connection …
koeninger Feb 4, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
methods to set consumer offsets and get topic metadata, switch back t…
…o inclusive start / exclusive end to match typical kafka consumer behavior
  • Loading branch information
koeninger committed Nov 24, 2014
commit 7d050bcb0bcacfbd4a7b858cffae809fd2af8e9d
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ package org.apache.spark.rdd.kafka
import scala.util.control.NonFatal
import scala.collection.mutable.ArrayBuffer
import java.util.Properties
import kafka.api.{OffsetRequest, OffsetResponse, OffsetFetchRequest, OffsetFetchResponse, PartitionOffsetRequestInfo, TopicMetadataRequest, TopicMetadataResponse}
import kafka.common.{ErrorMapping, TopicAndPartition}
import kafka.api.{OffsetCommitRequest, OffsetRequest, OffsetFetchRequest, PartitionOffsetRequestInfo, TopicMetadata, TopicMetadataRequest, TopicMetadataResponse}
import kafka.common.{ErrorMapping, OffsetMetadataAndError, TopicAndPartition}
import kafka.consumer.{ConsumerConfig, SimpleConsumer}

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you turn all scaladoc style into javadoc for this pr?

Expand Down Expand Up @@ -69,6 +69,27 @@ class KafkaCluster(val kafkaParams: Map[String, String]) {
Left(errs)
}

def getPartitions(topics: Set[String]): Either[Err, Set[TopicAndPartition]] =
getPartitionMetadata(topics).right.map { r =>
r.flatMap { tm: TopicMetadata =>
tm.partitionsMetadata.map { pm =>
TopicAndPartition(tm.topic, pm.partitionId)
}
}
}

def getPartitionMetadata(topics: Set[String]): Either[Err, Set[TopicMetadata]] = {
val req = TopicMetadataRequest(TopicMetadataRequest.CurrentVersion, 0, config.clientId, topics.toSeq)
val errs = new Err
withBrokers(errs) { consumer =>
val resp: TopicMetadataResponse = consumer.send(req)
// error codes here indicate missing / just created topic,
// repeating on a different broker wont be useful
return Right(resp.topicsMetadata.toSet)
}
Left(errs)
}

def getLatestLeaderOffsets(topicAndPartitions: Set[TopicAndPartition]): Either[Err, Map[TopicAndPartition, Long]] =
getLeaderOffsets(topicAndPartitions, OffsetRequest.LatestTime)

Expand All @@ -94,7 +115,7 @@ class KafkaCluster(val kafkaParams: Map[String, String]) {
)
val errs = new Err
withBrokers(errs) { consumer =>
val resp: OffsetResponse = consumer.getOffsetsBefore(req)
val resp = consumer.getOffsetsBefore(req)
val respMap = resp.partitionErrorAndOffsets
val needed = topicAndPartitions.diff(result.keys.toSet)
needed.foreach { tp =>
Expand All @@ -116,17 +137,28 @@ class KafkaCluster(val kafkaParams: Map[String, String]) {
}

def getConsumerOffsets(groupId: String, topicAndPartitions: Set[TopicAndPartition]): Either[Err, Map[TopicAndPartition, Long]] = {
var result = Map[TopicAndPartition, Long]()
getConsumerOffsetMetadata(groupId, topicAndPartitions).right.map { r =>
r.map { kv =>
kv._1 -> kv._2.offset
}
}
}

def getConsumerOffsetMetadata(
groupId: String,
topicAndPartitions: Set[TopicAndPartition]
): Either[Err, Map[TopicAndPartition, OffsetMetadataAndError]] = {
var result = Map[TopicAndPartition, OffsetMetadataAndError]()
val req = OffsetFetchRequest(groupId, topicAndPartitions.toSeq)
val errs = new Err
withBrokers(errs) { consumer =>
val resp: OffsetFetchResponse = consumer.fetchOffsets(req)
val resp = consumer.fetchOffsets(req)
val respMap = resp.requestInfo
val needed = topicAndPartitions.diff(result.keys.toSet)
needed.foreach { tp =>
respMap.get(tp).foreach { offsetMeta =>
if (offsetMeta.error == ErrorMapping.NoError) {
result += tp -> offsetMeta.offset
result += tp -> offsetMeta
} else {
errs.append(ErrorMapping.exceptionFor(offsetMeta.error))
}
Expand All @@ -141,7 +173,41 @@ class KafkaCluster(val kafkaParams: Map[String, String]) {
Left(errs)
}

def setConsumerOffsets(groupId: String, offsets: Map[TopicAndPartition, Long]): Unit = ???
def setConsumerOffsets(groupId: String, offsets: Map[TopicAndPartition, Long]): Unit = {
setConsumerOffsetMetadata(groupId, offsets.map { kv =>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do a { case (k,v) => vs accessing the tuples as ._1 etc

kv._1 -> OffsetMetadataAndError(kv._2)
})
}

def setConsumerOffsetMetadata(
groupId: String,
metadata: Map[TopicAndPartition, OffsetMetadataAndError]
): Either[Err, Map[TopicAndPartition, Short]] = {
var result = Map[TopicAndPartition, Short]()
val req = OffsetCommitRequest(groupId, metadata)
val errs = new Err
val topicAndPartitions = metadata.keys.toSet
withBrokers(errs) { consumer =>
val resp = consumer.commitOffsets(req)
val respMap = resp.requestInfo
val needed = topicAndPartitions.diff(result.keys.toSet)
needed.foreach { tp =>
respMap.get(tp).foreach { err =>
if (err == ErrorMapping.NoError) {
result += tp -> err
} else {
errs.append(ErrorMapping.exceptionFor(err))
}
}
}
if (result.keys.size == topicAndPartitions.size) {
return Right(result)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return? how about just
Right(result)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an early return from withBrokers, which would otherwise keep running the closure on each broker.

}
}
val missing = topicAndPartitions.diff(result.keys.toSet)
errs.append(new Exception(s"Couldn't set offsets for ${missing}"))
Left(errs)
}

private def withBrokers(errs: Err)(fn: SimpleConsumer => Any): Unit = {
brokers.foreach { hp =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ private[spark] case class KafkaRDDPartition(
override val index: Int,
topic: String,
partition: Int,
afterOffset: Long,
throughOffset: Long
fromOffset: Long,
untilOffset: Long
) extends Partition

/** A batch-oriented interface for consuming from Kafka.
Expand All @@ -46,8 +46,8 @@ private[spark] case class KafkaRDDPartition(
* @param kafkaParams Kafka <a href="http://kafka.apache.org/documentation.html#configuration">configuration parameters</a>.
* Requires "metadata.broker.list" or "bootstrap.servers" to be set with Kafka broker(s),
* NOT zookeeper servers, specified in host1:port1,host2:port2 form.
* @param afterOffsets per-topic/partition Kafka offsets defining the (exclusive) starting point of the batch
* @param throughOffsets per-topic/partition Kafka offsets defining the (inclusive) ending point of the batch
* @param fromOffsets per-topic/partition Kafka offsets defining the (inclusive) starting point of the batch
* @param untilOffsets per-topic/partition Kafka offsets defining the (exclusive) ending point of the batch
* @param messageHandler function for translating each message into the desired type
*/
class KafkaRDD[
Expand All @@ -58,71 +58,80 @@ class KafkaRDD[
R: ClassTag](
sc: SparkContext,
kafkaParams: Map[String, String],
afterOffsets: Map[TopicAndPartition, Long],
throughOffsets: Map[TopicAndPartition, Long],
fromOffsets: Map[TopicAndPartition, Long],
untilOffsets: Map[TopicAndPartition, Long],
messageHandler: MessageAndMetadata[K, V] => R
) extends RDD[R](sc, Nil) with Logging {

assert(afterOffsets.keys == throughOffsets.keys,
assert(fromOffsets.keys == untilOffsets.keys,
"Must provide both from and until offsets for each topic/partition")

override def getPartitions: Array[Partition] = afterOffsets.zipWithIndex.map { kvi =>
override def getPartitions: Array[Partition] = fromOffsets.zipWithIndex.map { kvi =>
val ((tp, from), index) = kvi
new KafkaRDDPartition(index, tp.topic, tp.partition, from, throughOffsets(tp))
new KafkaRDDPartition(index, tp.topic, tp.partition, from, untilOffsets(tp))
}.toArray

override def compute(thePart: Partition, context: TaskContext) = new NextIterator[R] {
context.addTaskCompletionListener{ context => closeIfNeeded() }

val kc = new KafkaCluster(kafkaParams)
override def compute(thePart: Partition, context: TaskContext) = {
val part = thePart.asInstanceOf[KafkaRDDPartition]
val keyDecoder = classTag[U].runtimeClass.getConstructor(classOf[VerifiableProperties])
.newInstance(kc.config.props)
.asInstanceOf[Decoder[K]]
val valueDecoder = classTag[T].runtimeClass.getConstructor(classOf[VerifiableProperties])
.newInstance(kc.config.props)
.asInstanceOf[Decoder[V]]
val consumer: SimpleConsumer = kc.connectLeader(part.topic, part.partition).fold(
errs => throw new Exception(s"""Couldn't connect to leader for topic ${part.topic} ${part.partition}: ${errs.mkString("\n")}"""),
consumer => consumer
)
var requestOffset = part.afterOffset + 1
var iter: Iterator[MessageAndOffset] = null
if (part.fromOffset >= part.untilOffset) {
log.warn(s"Beginning offset is same or after ending offset, skipping ${part.topic} ${part.partition}")
Iterator.empty
} else {
new NextIterator[R] {
context.addTaskCompletionListener{ context => closeIfNeeded() }

val kc = new KafkaCluster(kafkaParams)
log.info(s"Computing partition ${part.topic} ${part.partition} ${part.fromOffset} -> ${part.untilOffset}")
val keyDecoder = classTag[U].runtimeClass.getConstructor(classOf[VerifiableProperties])
.newInstance(kc.config.props)
.asInstanceOf[Decoder[K]]
val valueDecoder = classTag[T].runtimeClass.getConstructor(classOf[VerifiableProperties])
.newInstance(kc.config.props)
.asInstanceOf[Decoder[V]]
val consumer: SimpleConsumer = kc.connectLeader(part.topic, part.partition).fold(
errs => throw new Exception(s"""Couldn't connect to leader for topic ${part.topic} ${part.partition}: ${errs.mkString("\n")}"""),
consumer => consumer
)
var requestOffset = part.fromOffset
var iter: Iterator[MessageAndOffset] = null

override def getNext: R = {
if (iter == null || !iter.hasNext) {
val req = new FetchRequestBuilder().
addFetch(part.topic, part.partition, requestOffset, kc.config.fetchMessageMaxBytes).
build()
val resp = consumer.fetch(req)
if (resp.hasError) {
val err = resp.errorCode(part.topic, part.partition)
if (err == ErrorMapping.LeaderNotAvailableCode ||
err == ErrorMapping.NotLeaderForPartitionCode) {
log.error(s"Lost leader for topic ${part.topic} partition ${part.partition}, sleeping for ${kc.config.refreshLeaderBackoffMs}ms")
Thread.sleep(kc.config.refreshLeaderBackoffMs)
override def close() = consumer.close()

override def getNext: R = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getNext should have parentheses (since it has side effect and it was defined with parentheses in NextIterator)

if (iter == null || !iter.hasNext) {
log.info(s"Fetching ${part.topic}, ${part.partition}, ${requestOffset}")
val req = new FetchRequestBuilder().
addFetch(part.topic, part.partition, requestOffset, kc.config.fetchMessageMaxBytes).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move the dot to next line for line 125 and line 126 to be consistent

build()
val resp = consumer.fetch(req)
if (resp.hasError) {
val err = resp.errorCode(part.topic, part.partition)
if (err == ErrorMapping.LeaderNotAvailableCode ||
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have 3 nested if statements here, I'd break something out to it's own private function

err == ErrorMapping.NotLeaderForPartitionCode) {
log.error(s"Lost leader for topic ${part.topic} partition ${part.partition}, sleeping for ${kc.config.refreshLeaderBackoffMs}ms")
Thread.sleep(kc.config.refreshLeaderBackoffMs)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why Thread.sleep? I would not want to use that in an async app.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's only happening in an error case, and is being done to make sure that you don't burn through the maximum number of failed stages while leader election is happening.

I'm pretty sure the existing spark kafka dstreams are doing the exact same thing, because that's what the kafka project's consumer code does in ConsumerFetcherManager.

Do you have an alternative suggestion?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not without taking time to review the related code but Thread.sleep in async is a no IMHO and in every project I've worked in for the last 6 years (all scala async envs). But I don't ATM.

}
// Let normal rdd retry sort out reconnect attempts
throw ErrorMapping.exceptionFor(err)
}
iter = resp.messageSet(part.topic, part.partition)
.iterator
.dropWhile(_.offset < requestOffset)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is there a drop here? Doesnt the response return messages for the requested offset?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://cwiki.apache.org/confluence/display/KAFKA/0.8.0+SimpleConsumer+Example

"Also note that we are explicitly checking that the offset being read is
not less than the offset that we requested. This is needed since if Kafka
is compressing the messages, the fetch request will return an entire
compressed block even if the requested offset isn't the beginning of the
compressed block. Thus a message we saw previously may be returned again."

On Tue, Dec 30, 2014 at 2:00 PM, Tathagata Das [email protected]
wrote:

In external/kafka/src/main/scala/org/apache/spark/rdd/kafka/KafkaRDD.scala
#3798 (diff):

  •          build()
    
  •        val resp = consumer.fetch(req)
    
  •        if (resp.hasError) {
    
  •          val err = resp.errorCode(part.topic, part.partition)
    
  •          if (err == ErrorMapping.LeaderNotAvailableCode ||
    
  •            err == ErrorMapping.NotLeaderForPartitionCode) {
    
  •            log.error(s"Lost leader for topic ${part.topic} partition ${part.partition}, " +
    
  •              s" sleeping for ${kc.config.refreshLeaderBackoffMs}ms")
    
  •            Thread.sleep(kc.config.refreshLeaderBackoffMs)
    
  •          }
    
  •          // Let normal rdd retry sort out reconnect attempts
    
  •          throw ErrorMapping.exceptionFor(err)
    
  •        }
    
  •        iter = resp.messageSet(part.topic, part.partition)
    
  •          .iterator
    
  •          .dropWhile(_.offset < requestOffset)
    

Why is there a drop here? Doesnt the response return messages for the
requested offset?


Reply to this email directly or view it on GitHub
https://github.com/apache/spark/pull/3798/files#r22362167.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow! That was not intuitive. Worth mentioning this in the code.

}
if (!iter.hasNext) {
finished = true
null.asInstanceOf[R]
} else {
val item = iter.next
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

next should have parentheses

if (item.offset > part.untilOffset) {
finished = true
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this level of conditional / structure nesting seems scary. Any chance we can reduce this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that's one of the ugliest parts of the code, but unfortunately I
think those are the cases that need to be handled. I'll see if there's a
way to flatten it.

On Mon, Jan 26, 2015 at 5:06 PM, Reynold Xin [email protected]
wrote:

In external/kafka/src/main/scala/org/apache/spark/rdd/kafka/KafkaRDD.scala
#3798 (comment):

  •        finished = true
    
  •        null.asInstanceOf[R]
    
  •      } else {
    
  •        val item = iter.next
    
  •        if (item.offset >= part.untilOffset) {
    
  •          assert(item.offset == part.untilOffset,
    
  •            s"got ${item.offset} > ending offset ${part.untilOffset} " +
    
  •              s"for topic ${part.topic} partition ${part.partition} start ${part.fromOffset}." +
    
  •              " This should not happen, and indicates a message may have been skipped")
    
  •          finished = true
    
  •          null.asInstanceOf[R]
    
  •        } else {
    
  •          requestOffset = item.nextOffset
    
  •          messageHandler(new MessageAndMetadata(
    
  •            part.topic, part.partition, item.message, item.offset, keyDecoder, valueDecoder))
    
  •        }
    

this level of conditional / structure nesting seems scary. Any chance we
can reduce this?


Reply to this email directly or view it on GitHub
https://github.com/apache/spark/pull/3798/files#r23573027.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw one way you can reduce is to use return to help your control flow. for example, if part.fromOffset >= part.untilOffset, you can explicitly return an iterator. That reduces one level. Many other ways you can do this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good way is to define subfunctions within meaningful names. Then the complex condition logic can broken down and will be easier to read.

requestOffset = item.nextOffset
messageHandler(new MessageAndMetadata(part.topic, part.partition, item.message, item.offset, keyDecoder, valueDecoder))
}
// Let normal rdd retry sort out reconnect attempts
throw ErrorMapping.exceptionFor(err)
}
iter = resp.messageSet(part.topic, part.partition)
.iterator
.dropWhile(_.offset < requestOffset)
}
if (!iter.hasNext) {
finished = true
null.asInstanceOf[R]
} else {
val item = iter.next
if (item.offset > part.throughOffset) {
finished = true
}
requestOffset = item.nextOffset
messageHandler(new MessageAndMetadata(part.topic, part.partition, item.message, item.offset, keyDecoder, valueDecoder))
}
}

override def close() = consumer.close()
}

}