incubator-kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jay Kreps <jay.kr...@gmail.com>
Subject Re: HighwaterMarkCheckpoint
Date Tue, 27 Nov 2012 01:20:52 GMT
Also I don't see any unit test coverage for this class directly, is that
right? If so I will add it...

I do see two big integration tests HighwatermarkPersistenceTest and
LogRecoveryTest both use it. What is the difference between these?

-Jay


On Mon, Nov 26, 2012 at 5:17 PM, Jay Kreps <jay.kreps@gmail.com> wrote:

> Does this capture the intention of this code:
> https://gist.github.com/4151743
> ?
>
> The idea is to rename it to OffsetCheckpoint so it is generic. To do this
> 1. I make the file name and argument
> 2. read now returns a Map of all values. To get the old behavior you would
> do chkpt.read.getOrElse(TopicPartition(t,p), 0L). This makes sense as
> defaulting to 0L may only make sense for highwatermarks.
> 3. Throw and exception rather than calling shutdown (always a good idea)
>
> Some things I am confused about:
> 1. Parsing logic--there is very complex being done but I don't know why.
> Are we just parsing three space-separated values? Why all the indexOf stuff?
> 2. Why do we store the number of entries in the file?
>
> Cheers,
>
> -Jay
>
>
> On Mon, Nov 26, 2012 at 4:14 PM, Jay Kreps <jay.kreps@gmail.com> wrote:
>
>> The api for the HighwaterMarkCheckpoint is:
>>
>>   def write(highwaterMarksPerPartition: Map[TopicAndPartition, Long])
>>
>>   def read(topic: String, partition: Int): Long
>>
>> Pretty weird, no? The write method writes a map of all values,
>> obliterating previous values. The read method internally reads back that
>> whole map, then throws away all the values but one and returns that one. It
>> seems like the natural way to do it would be to either read and write one
>> key/value pair or read and write them all together.
>>
>> Is there a rationale for this? Can we change it?
>>
>> It would be nice to reuse this class to track log flushes, and the
>> cleaner point. However in its current form it isn't particularly useful.
>>
>> -Jay
>>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message