> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > Nice work! It's a big patch, but it makes sense. Definitely excited to have 
> > this part of Samza.
> > 
> > The comments below are mostly stylistic/clarity things. Just one big 
> > question I want to specifically call out:
> > 
> > The checkpoint topic is changing in an incompatible way, which means that 
> > any Samza 0.8.0 jobs won't be able to read Samza 0.7.0 checkpoints and vice 
> > versa. What should we do about this?
> > 
> > While I think it's ok to break API compatibility between versions (code is 
> > fairly easy to change), I'm a bit hesitant about an incompatible data 
> > change, as it will create a deployment headache for anyone upgrading their 
> > job from Samza 0.7.0 to 0.8.0. They will have to switch to a new checkpoint 
> > topic (since a 0.8.0 job will fail to start when trying to read a 0.7.0 
> > checkpoint). We would probably need to provide a checkpoint conversion 
> > tool, otherwise everybody would lose their checkpoints. This 
> > incompatibility occurs even if the user isn't taking advantage of the new 
> > SSP grouping feature.
> > 
> > Should we provide a smoother upgrade path here? I would say that, as a 
> > minimum, 0.8.0 should be able to read 0.7.0-style checkpoints and 
> > automatically convert them into 0.8.0 format.

I'm more willing to force the change.  Those needing to keep the old 
checkpoints can keep using the old version.  Also, in-place conversion of the 
log/support for the old style checkpoint log would be huge.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-api/src/main/java/org/apache/samza/checkpoint/Checkpoint.java, line 62
> > <https://reviews.apache.org/r/22215/diff/2/?file=605716#file605716line62>
> >
> >     Nit: use "o instanceof Checkpoint" rather than "getClass() != 
> > getClass()" to allow potential subclassing of Checkpoint?

fixed


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-api/src/main/java/org/apache/samza/checkpoint/CheckpointManager.java, 
> > line 55
> > <https://reviews.apache.org/r/22215/diff/2/?file=605717#file605717line55>
> >
> >     On first read, it wasn't clear to me what partitions this referred to. 
> > First I thought it was input stream partitions, then I thought it was 
> > partitions of the checkpoint stream.
> >     
> >     Could you update the docs to make more explicit that this is about 
> > partitions of changelog streams for stores?

Renamed this to statelog partition mapping everywhere to be more explicit.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-api/src/main/java/org/apache/samza/container/SSPGrouper.java, line 29
> > <https://reviews.apache.org/r/22215/diff/2/?file=605718#file605718line29>
> >
> >     I don't understand this sentence: "Each taskName has a key that 
> > uniquely describes what sets may be in it, but does not generally enumerate 
> > the elements of those sets." Could you clarify please?

Added more explanation.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-api/src/main/java/org/apache/samza/container/SamzaContainerContext.java,
> >  line 40
> > <https://reviews.apache.org/r/22215/diff/2/?file=605720#file605720line40>
> >
> >     What's the difference between a taskName and a taskName key? My 
> > understanding was that a taskName is a string that identifies a particular 
> > task instance within a job (e.g. with the current Samza behavior, the 
> > taskName could be the partition number).

None.  Fixed also from Chris' comment.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/checkpoint/CheckpointTool.scala, 
> > line 141
> > <https://reviews.apache.org/r/22215/diff/2/?file=605724#file605724line141>
> >
> >     Is there a JIRA for adapting CheckpointTool to the new checkpoint 
> > format?

Not yet.  With the latest way of handling the checkpoint manager, this should 
be a quick fix to include in this patch, assuming we're all ok on the new 
checkpoint manager approach.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/checkpoint/OffsetManager.scala, 
> > line 197
> > <https://reviews.apache.org/r/22215/diff/2/?file=605725#file605725line197>
> >
> >     Javadoc needs updating to match the code.

done.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/checkpoint/file/FileSystemCheckpointManager.scala,
> >  line 80
> > <https://reviews.apache.org/r/22215/diff/2/?file=605726#file605726line80>
> >
> >     Perhaps use different namespaces for checkpoints and the partition 
> > mapping, to remove the risk of filename collision? e.g. checkpoints go in
> >     
> >     new File(root, "%s-task-%s" format (jobName, taskName))
> >     
> >     and partition mapping goes in
> >     
> >     new File(root, "%s-partition-mapping" format jobName)

done.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/checkpoint/file/FileSystemCheckpointManager.scala,
> >  line 100
> > <https://reviews.apache.org/r/22215/diff/2/?file=605726#file605726line100>
> >
> >     Nit: the two equals signs in the same line caught me by surprise. I'd 
> > find this more readable:
> >     
> >     override def setTaskNameToPartitionMapping(mapping: util.Map[String, 
> > Integer]) {
> >       taskNameMapping = mapping
> >     }

removed as part of the checkpoint manager change.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/container/SSPTaskNameGrouper.scala,
> >  line 21
> > <https://reviews.apache.org/r/22215/diff/2/?file=605730#file605730line21>
> >
> >     Could you add docs for SSPTaskNameGrouper? Its purpose is not obvious 
> > to me.

added lots more java doc to the interface.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala, 
> > line 78
> > <https://reviews.apache.org/r/22215/diff/2/?file=605731#file605731line78>
> >
> >     It looks like Util.decodeTaskNameToPartitionMapping converts from Java 
> > types to Scala types, and then here you convert straight back again. Would 
> > it be easier to just keep the mapping in Java types throughout?

Unfortunately no.  The problem is that it's a scala.Map[TaskName, Int] to 
java.Map[TaskName, Integer].  One can't easily map over the latter, but one 
cannot do automatic conversions because you need to convert both the Int <-> 
Integer and java.Map<->scala.Map


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala, 
> > line 426
> > <https://reviews.apache.org/r/22215/diff/2/?file=605731#file605731line426>
> >
> >     Nit: could simplify to: sspTaskNames.getOrElse(taskName, throw ...)

done.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/container/ssp/groupers/GroupByPartition.scala,
> >  line 29
> > <https://reviews.apache.org/r/22215/diff/2/?file=605735#file605735line29>
> >
> >     Nit: unnecessary curly braces? Also, quite a long line -- perhaps break 
> > after equals sign?

done.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/serializers/CheckpointSerde.scala,
> >  line 41
> > <https://reviews.apache.org/r/22215/diff/2/?file=605741#file605741line41>
> >
> >     Perhaps a radical and/or stupid suggestion, but just to throw it out 
> > there... if JSON is causing pain, and we're switching to a different 
> > serialization for checkpoints anyway, we could potentially use something 
> > like Avro instead of JSON.
> >     
> >     With Avro, we could specify a schema for the data we want (a 
> > checkpoint, in this case), and code-generate Java classes for accessing the 
> > data. They ought to be more Scala-friendly (less maps and dynamic typing). 
> > Avro also gives us a good system for schema evolution if we want to change 
> > aspects of the serialization in future.
> >     
> >     Not pushing Avro, just throwing it out there as an idea.

The problem isn't JSON, the problem is the Scala/Java conversions.  If this 
were all one or the other, there would be no issues.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/serializers/CheckpointSerde.scala,
> >  line 47
> > <https://reviews.apache.org/r/22215/diff/2/?file=605741#file605741line47>
> >
> >     This serde will not be able to read checkpoints written by a previous 
> > version of Samza -- is that right? That would set up quite a hurdle for 
> > people migrating from 0.7.0 to 0.8.0.
> >     
> >     I wonder if it would be possible to at least support reading the old 
> > checkpoint format, and converting it into the new format (assuming that 
> > most people will probably stick with the default GroupByPartition for now). 
> > Downgrading back from 0.8.0 to 0.7.0 wouldn't be possible, but at least the 
> > upgrade path could be smooth.
> >     
> >     (We can add the compatibility in a separate JIRA if we want to.)
> >     
> >     As we're already changing the checkpoint format, I also wonder if we 
> > could build some extensibility into it -- eg. by using JSON objects and 
> > ignoring any keys that are not recognized. In case we find ourselves 
> > needing to add further information to checkpoints in future (for example, 
> > in order to provide exactly-once semantics), we could do that by evolving 
> > the format.

Correct.
We can certainly write such a tool


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/serializers/CheckpointSerde.scala,
> >  line 58
> > <https://reviews.apache.org/r/22215/diff/2/?file=605741#file605741line58>
> >
> >     Left-over debug statement?

fixed.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/serializers/CheckpointSerde.scala,
> >  line 93
> > <https://reviews.apache.org/r/22215/diff/2/?file=605741#file605741line93>
> >
> >     Would it make sense to have a separate serde for the partition mapping, 
> > rather than tacking it on at the side of CheckpointSerde?

There are comments in the code newly added about this.  Since this is an 
intermediate step from the whole state log, I don't want to do much breakage 
here.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/serializers/CheckpointSerde.scala,
> >  line 98
> > <https://reviews.apache.org/r/22215/diff/2/?file=605741#file605741line98>
> >
> >     Left-over debug statement?

fixed


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/serializers/CheckpointSerde.scala,
> >  line 100
> > <https://reviews.apache.org/r/22215/diff/2/?file=605741#file605741line100>
> >
> >     The exception needs to be the second argument to debug() -- see 
> > SAMZA-275.

fixed


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/util/Util.scala, line 122
> > <https://reviews.apache.org/r/22215/diff/2/?file=605744#file605744line122>
> >
> >     This Util class would probably benefit from some refactoring. I find 
> > util classes are always a bit of a "couldn't think of a proper name for 
> > this stuff" thing, but this one contains lots of important logic that 
> > really deserves classes of its own.

Agree.  Would like to do it in another JIRA.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/util/Util.scala, line 128
> > <https://reviews.apache.org/r/22215/diff/2/?file=605744#file605744line128>
> >
> >     Could import GroupByPartitionFactory to shorten this.

Fixed.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/util/Util.scala, line 146
> > <https://reviews.apache.org/r/22215/diff/2/?file=605744#file605744line146>
> >
> >     Is this using the YARN terminology for "task", i.e. container? The 
> > Samza documentation refers to them as containers, and uses "task" only with 
> > the Samza meaning of "TaskInstance" (in SAMZA-7 I've tried to use "task 
> > instance" more consistently instead of just "task").
> >     
> >     I would advocate that in Samza we consistently refer to the units of 
> > parallelism as "containers", not "tasks". In the code directly interfacing 
> > with YARN we might not be able to avoid YARN's definition of "task", but in 
> > the rest of our code (including here) I think it would be a lot less 
> > confusing if we stick to "container".
> >     
> >     The same goes for log messages (users reading the logs will assume that 
> > if we say "task", we mean "task instance").
> >     
> >     What do you think?

Changed this a bit to use container as I could, but we use Task pretty much 
everywhere.  It's worse because we actually call them TaskGroup in the web 
interface.  This should be fixed in a larger JIRA.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/util/Util.scala, line 202
> > <https://reviews.apache.org/r/22215/diff/2/?file=605744#file605744line202>
> >
> >     Fair enough to fully qualify the Java types, but SystemStreamPartition 
> > is already imported.

fixed.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/util/Util.scala, line 225
> > <https://reviews.apache.org/r/22215/diff/2/?file=605744#file605744line225>
> >
> >     Kinda similar to CheckpointSerde.partitionMappingToBytes but not quite?

Happens to be the same, but as an implementation detail.  There's no reason we 
need to keep writing the checkpoint content out as JSON, so I'd like not 
intermingle this code.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/util/Util.scala, line 249
> > <https://reviews.apache.org/r/22215/diff/2/?file=605744#file605744line249>
> >
> >     There's some reasonably subtle logic happening here. Would be good to 
> > have thorough unit test coverage.

I had actually thought this was one of the more functional pieces of code since 
we're basically just doing set difference/addition.  Added more comments.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/main/scala/org/apache/samza/util/Util.scala, line 257
> > <https://reviews.apache.org/r/22215/diff/2/?file=605744#file605744line257>
> >
> >     This could also happen if checkpointing is disabled. Which reminds me: 
> > we should probably disallow jobs with state stores but no checkpoint topic, 
> > since that configuration would make a deterministic assignment of taskNames 
> > to changelog partitions impossible. Not sure where the best place would be 
> > for such a check.

Yeah, we should add this check but I also can't figure out the best place.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/test/scala/org/apache/samza/checkpoint/file/TestFileSystemCheckpointManager.scala,
> >  line 35
> > <https://reviews.apache.org/r/22215/diff/2/?file=605747#file605747line35>
> >
> >     Variable name could be a little more descriptive ;)

Fixed.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/test/scala/org/apache/samza/serializers/TestCheckpointSerde.scala,
> >  line 31
> > <https://reviews.apache.org/r/22215/diff/2/?file=605755#file605755line31>
> >
> >     Could do with more tests here (or is there a follow-up jira for that?).

So there were previously only two tests, one of which no longer made sense and 
was deleted.  We should definitely add more test in a follow-up JIRA.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-core/src/test/scala/org/apache/samza/util/TestUtil.scala, line 106
> > <https://reviews.apache.org/r/22215/diff/2/?file=605757#file605757line106>
> >
> >     Forgotten debug statement?

fixed.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala,
> >  line 138
> > <https://reviews.apache.org/r/22215/diff/2/?file=605758#file605758line138>
> >
> >     I believe the last argument should be -1 
> > (kafka.api.Request.OrdinaryConsumerId) 
> > https://git-wip-us.apache.org/repos/asf?p=kafka.git;a=blob;f=core/src/main/scala/kafka/api/RequestOrResponse.scala;h=57f87a48c5e87220e7f377b23d2bbfa0d16350dc;hb=HEAD#l24
> >  but the Kafka team can probably tell us for sure.

Set to -1 and will check with the Kafka team.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala,
> >  line 140
> > <https://reviews.apache.org/r/22215/diff/2/?file=605758#file605758line140>
> >
> >     What is the difference between getEarliestOffset(c, tp) and 
> > getOffset(c, tp, OffsetRequest.EarliestTime)? They seem to do the same 
> > thing.

The former was meant as a convenience method but could be consolidated.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala,
> >  line 197
> > <https://reviews.apache.org/r/22215/diff/2/?file=605758#file605758line197>
> >
> >     Do you know in which circumstances this could happen? Perhaps 
> > OffsetOutOfRange can no longer happen when you're consuming the full 
> > checkpoint topic from beginning to end?

It might happen if the checkpoint is so old it's fallen off the end of the 
topic.  It could still theoretically happen but very, very unlikely.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala,
> >  line 203
> > <https://reviews.apache.org/r/22215/diff/2/?file=605758#file605758line203>
> >
> >     This could potentially be simplified by using 
> > SystemStreamPartitionIterator (which is used for consuming the entirety of 
> > a changelog partition in order to restore a state store) instead of the 
> > low-level Kafka API.

Let's do that in a follow up.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala,
> >  line 251
> > <https://reviews.apache.org/r/22215/diff/2/?file=605758#file605758line251>
> >
> >     On container/job startup, when readLastCheckpoint is called several 
> > times in quick succession for different task names, readCheckpoint() will 
> > be called for each task name, which looks like it could be wasteful. 
> > Although readCheckpoint() won't re-consume the entire topic (because it 
> > maintains the offset up to which it has read), it looks like it will still 
> > make a request to Kafka to get the latest offset every time 
> > readCheckpoint() is called.
> >     
> >     Perhaps that can be avoided, eg. by only connecting to Kafka if some 
> > minimum time has elapsed since we last checked the latest offset?

I'm hoping to move the consumer-stored checkpoints very soon and eliminate the 
whole reading, and even sooner, move the reading to the AM.  I'd like to keep 
it simpler for now and delete soon.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala,
> >  line 350
> > <https://reviews.apache.org/r/22215/diff/2/?file=605758#file605758line350>
> >
> >     A lot of duplicated code between this and readCheckpoint() above. Would 
> > be good to factor out the common parts.

I've actually duplicated even more in the latest patch on the hope to move to 
the new state log as soon as possible.  I tried refactoring a couple times and 
it's quite subtle where the code is different.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala,
> >  line 408
> > <https://reviews.apache.org/r/22215/diff/2/?file=605758#file605758line408>
> >
> >     The exception needs to be the second argument to debug() -- see 
> > SAMZA-275.

fixed.


> On June 11, 2014, 8:32 a.m., Martin Kleppmann wrote:
> > samza-yarn/src/main/scala/org/apache/samza/webapp/ApplicationMasterRestServlet.scala,
> >  line 90
> > <https://reviews.apache.org/r/22215/diff/2/?file=605768#file605768line90>
> >
> >     The other properties use hyphens instead of spaces; although a space is 
> > valid, a hyphen might be better for consistency.

fixed.


- Jakob


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22215/#review45235
-----------------------------------------------------------


On July 3, 2014, 12:50 p.m., Jakob Homan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22215/
> -----------------------------------------------------------
> 
> (Updated July 3, 2014, 12:50 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Bugs: SAMZA-123
>     https://issues.apache.org/jira/browse/SAMZA-123
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> Move topic partition grouping to the AM and generalize
> 
> 
> Diffs
> -----
> 
>   .gitignore db9d3ec 
>   samza-api/src/main/java/org/apache/samza/checkpoint/Checkpoint.java 6fad1fa 
>   samza-api/src/main/java/org/apache/samza/checkpoint/CheckpointManager.java 
> a6e1ba6 
>   
> samza-api/src/main/java/org/apache/samza/container/SamzaContainerContext.java 
> 78d56a9 
>   
> samza-api/src/main/java/org/apache/samza/container/SystemStreamPartitionGrouper.java
>  PRE-CREATION 
>   
> samza-api/src/main/java/org/apache/samza/container/SystemStreamPartitionGrouperFactory.java
>  PRE-CREATION 
>   samza-api/src/main/java/org/apache/samza/container/TaskName.java 
> PRE-CREATION 
>   samza-api/src/main/java/org/apache/samza/job/CommandBuilder.java cb40092 
>   samza-api/src/main/java/org/apache/samza/task/TaskContext.java 7c1b085 
>   samza-core/src/main/scala/org/apache/samza/checkpoint/CheckpointTool.scala 
> 5735a39 
>   samza-core/src/main/scala/org/apache/samza/checkpoint/OffsetManager.scala 
> 9487b58 
>   
> samza-core/src/main/scala/org/apache/samza/checkpoint/file/FileSystemCheckpointManager.scala
>  364e489 
>   samza-core/src/main/scala/org/apache/samza/config/JobConfig.scala fcafe83 
>   samza-core/src/main/scala/org/apache/samza/config/ShellCommandConfig.scala 
> 4c2d365 
>   samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala 4ca340c 
>   samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 
> 356adbb 
>   
> samza-core/src/main/scala/org/apache/samza/container/SystemStreamPartitionTaskNameGrouper.scala
>  PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/container/TaskInstance.scala 
> 99a9841 
>   
> samza-core/src/main/scala/org/apache/samza/container/TaskInstanceMetrics.scala
>  7502124 
>   
> samza-core/src/main/scala/org/apache/samza/container/TaskNamesToSystemStreamPartitions.scala
>  PRE-CREATION 
>   
> samza-core/src/main/scala/org/apache/samza/container/ssp/groupers/GroupByPartition.scala
>  PRE-CREATION 
>   
> samza-core/src/main/scala/org/apache/samza/container/ssp/groupers/GroupBySystemStreamPartition.scala
>  PRE-CREATION 
>   
> samza-core/src/main/scala/org/apache/samza/container/ssp/taskname/groupers/SimpleSystemStreamPartitionTaskNameGrouper.scala
>  PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/job/ShellCommandBuilder.scala 
> f8865b1 
>   samza-core/src/main/scala/org/apache/samza/job/local/LocalJobFactory.scala 
> e20e7c1 
>   
> samza-core/src/main/scala/org/apache/samza/serializers/CheckpointSerde.scala 
> 3d0a484 
>   samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala 
> 7214151 
>   samza-core/src/main/scala/org/apache/samza/task/ReadableCoordinator.scala 
> 4ccd604 
>   samza-core/src/main/scala/org/apache/samza/util/Util.scala 11c23d0 
>   
> samza-core/src/test/scala/org/apache/samza/checkpoint/TestCheckpointTool.scala
>  bc54f9e 
>   
> samza-core/src/test/scala/org/apache/samza/checkpoint/TestOffsetManager.scala 
> 94f6f4c 
>   
> samza-core/src/test/scala/org/apache/samza/checkpoint/file/TestFileSystemCheckpointManager.scala
>  50d9a05 
>   
> samza-core/src/test/scala/org/apache/samza/container/SystemStreamPartitionGrouperTestBase.scala
>  PRE-CREATION 
>   samza-core/src/test/scala/org/apache/samza/container/TestRunLoop.scala 
> fa10231 
>   
> samza-core/src/test/scala/org/apache/samza/container/TestSamzaContainer.scala 
> 190bdfe 
>   samza-core/src/test/scala/org/apache/samza/container/TestTaskInstance.scala 
> 1f5e3bb 
>   
> samza-core/src/test/scala/org/apache/samza/container/ssp/groupers/TestGroupByPartition.scala
>  PRE-CREATION 
>   
> samza-core/src/test/scala/org/apache/samza/container/ssp/groupers/TestGroupBySystemStreamPartition.scala
>  PRE-CREATION 
>   samza-core/src/test/scala/org/apache/samza/job/TestJobRunner.scala 21d8a78 
>   samza-core/src/test/scala/org/apache/samza/metrics/TestJmxServer.scala 
> 4f7ddcd 
>   
> samza-core/src/test/scala/org/apache/samza/serializers/TestCheckpointSerde.scala
>  70d8c80 
>   
> samza-core/src/test/scala/org/apache/samza/task/TestReadableCoordinator.scala 
> 12f1e03 
>   samza-core/src/test/scala/org/apache/samza/util/TestUtil.scala a67ecdf 
>   
> samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala
>  15245d4 
>   
> samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManagerFactory.scala
>  cb6dbdf 
>   
> samza-kafka/src/test/scala/org/apache/samza/checkpoint/kafka/TestKafkaCheckpointManager.scala
>  92ac61e 
>   
> samza-kafka/src/test/scala/org/apache/samza/system/kafka/TestKafkaSystemAdmin.scala
>  6be9732 
>   
> samza-kv/src/main/scala/org/apache/samza/storage/kv/LevelDbKeyValueStore.scala
>  dae3c2c 
>   
> samza-test/src/main/java/org/apache/samza/test/integration/join/Emitter.java 
> 222c130 
>   
> samza-test/src/main/scala/org/apache/samza/test/performance/TestKeyValuePerformance.scala
>  0077af0 
>   
> samza-test/src/test/scala/org/apache/samza/test/integration/TestStatefulTask.scala
>  dc44a99 
>   
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterState.scala 
> 01a2683 
>   
> samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterTaskManager.scala
>  eb1ff54 
>   
> samza-yarn/src/main/scala/org/apache/samza/webapp/ApplicationMasterRestServlet.scala
>  520f784 
>   
> samza-yarn/src/test/scala/org/apache/samza/job/yarn/TestSamzaAppMasterTaskManager.scala
>  f1139f5 
> 
> Diff: https://reviews.apache.org/r/22215/diff/
> 
> 
> Testing
> -------
> 
> Existing and new unit.  Now moving on to function.
> 
> 
> Thanks,
> 
> Jakob Homan
> 
>

Reply via email to