You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com> on 2015/09/22 10:20:03 UTC

Review Request 38614: SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints

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

Review request for samza, Yan Fang and Navina Ramesh.


Bugs: SAMZA-615
    https://issues.apache.org/jira/browse/SAMZA-615


Repository: samza


Description
-------

SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints


Diffs
-----

  build.gradle 3a7fabcd4f4e5c8db950c3a33bba33618c5565b4 
  checkstyle/import-control.xml bc07ae8116c00aecf2ac2fa3da58985c5ebe9e11 
  samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemConsumer.java 79291af64a83e59901de648ca2c81c8317235dc1 
  samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemProducer.java 42ae00b3166b8adc8ad0ce102e1707d52946dc2e 
  samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/CoordinatorStreamMessage.java 68e50a12eb0c7d5d66a11da48cc28c5b2dfa7b87 
  samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetMigrationMetaMessage.java PRE-CREATION 
  samza-core/src/main/java/org/apache/samza/migration/MigrationPlan.java PRE-CREATION 
  samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala 16732179f8f3783a62b1bb4f34ce46da44a3140e 
  samza-core/src/main/scala/org/apache/samza/migration/JobRunnerMigration.scala PRE-CREATION 
  samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamSystemFactory.java 84ae0b5e3d35c6cabf7f712610d2118c8982bd1b 
  samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamWrappedConsumer.java 47a44b1df3f3abdc782ca1b29c520609265fd365 
  samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamMessage.java 60e2e5df65db5435c47ea525d85dee451279c798 
  samza-core/src/test/java/org/apache/samza/storage/TestStorageRecovery.java 52d450d79cc1f87fe43dcd9ef06dbed4432fa919 
  samza-core/src/test/resources/test-migration-fail.properties PRE-CREATION 
  samza-core/src/test/scala/org/apache/samza/job/TestJobRunner.scala 52057edbbcd08b7d6edde7e898466d26534f52f6 
  samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointLogKey.scala PRE-CREATION 
  samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManager.scala PRE-CREATION 
  samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManagerFactory.scala PRE-CREATION 
  samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointMigration.scala PRE-CREATION 
  samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemProducer.scala c0c34bf887d3c0bc4beb8a06a1897a82bba0c654 
  samza-kafka/src/test/scala/old/checkpoint/TestKafkaCheckpointManager.scala PRE-CREATION 

Diff: https://reviews.apache.org/r/38614/diff/


Testing
-------


Thanks,

Yi Pan (Data Infrastructure)


Re: Review Request 38614: SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints

Posted by Navina Ramesh <nr...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38614/#review99969
-----------------------------------------------------------

Ship it!


Looks good to me. I hope you were able to test with a sample job.


samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManager.scala (line 81)
<https://reviews.apache.org/r/38614/#comment157023>

    If the write* methods are used only in unit tests, should we consider moving it to TestKafkaCheckpointManager class?



samza-kafka/src/test/scala/old/checkpoint/TestKafkaCheckpointManager.scala (line 307)
<https://reviews.apache.org/r/38614/#comment157021>

    Remove unused commented line


- Navina Ramesh


On Sept. 22, 2015, 11 p.m., Yi Pan (Data Infrastructure) wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38614/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2015, 11 p.m.)
> 
> 
> Review request for samza, Yan Fang and Navina Ramesh.
> 
> 
> Bugs: SAMZA-615
>     https://issues.apache.org/jira/browse/SAMZA-615
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints
> 
> 
> Diffs
> -----
> 
>   build.gradle 3a7fabcd4f4e5c8db950c3a33bba33618c5565b4 
>   checkstyle/import-control.xml bc07ae8116c00aecf2ac2fa3da58985c5ebe9e11 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemConsumer.java 79291af64a83e59901de648ca2c81c8317235dc1 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemProducer.java 42ae00b3166b8adc8ad0ce102e1707d52946dc2e 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetMigrationMetaMessage.java PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/migration/MigrationPlan.java PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala 16732179f8f3783a62b1bb4f34ce46da44a3140e 
>   samza-core/src/main/scala/org/apache/samza/migration/JobRunnerMigration.scala PRE-CREATION 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamSystemFactory.java 84ae0b5e3d35c6cabf7f712610d2118c8982bd1b 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamWrappedConsumer.java 47a44b1df3f3abdc782ca1b29c520609265fd365 
>   samza-core/src/test/java/org/apache/samza/storage/TestStorageRecovery.java 52d450d79cc1f87fe43dcd9ef06dbed4432fa919 
>   samza-core/src/test/resources/test-migration-fail.properties PRE-CREATION 
>   samza-core/src/test/scala/org/apache/samza/job/TestJobRunner.scala 52057edbbcd08b7d6edde7e898466d26534f52f6 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointLogKey.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManager.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManagerFactory.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointMigration.scala PRE-CREATION 
>   samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemProducer.scala c0c34bf887d3c0bc4beb8a06a1897a82bba0c654 
>   samza-kafka/src/test/scala/old/checkpoint/TestKafkaCheckpointManager.scala PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38614/diff/
> 
> 
> Testing
> -------
> 
> ./bin/check-all.sh passed.
> 
> 
> Thanks,
> 
> Yi Pan (Data Infrastructure)
> 
>


Re: Review Request 38614: SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints

Posted by Navina Ramesh <nr...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38614/#review100177
-----------------------------------------------------------

Ship it!


I am not completely happy with the "old.checkpoint" package. But looks like that it is easier than moving it. 
Otherwise, looks good to me!

- Navina Ramesh


On Sept. 22, 2015, 11 p.m., Yi Pan (Data Infrastructure) wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38614/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2015, 11 p.m.)
> 
> 
> Review request for samza, Yan Fang and Navina Ramesh.
> 
> 
> Bugs: SAMZA-615
>     https://issues.apache.org/jira/browse/SAMZA-615
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints
> 
> 
> Diffs
> -----
> 
>   build.gradle 3a7fabcd4f4e5c8db950c3a33bba33618c5565b4 
>   checkstyle/import-control.xml bc07ae8116c00aecf2ac2fa3da58985c5ebe9e11 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemConsumer.java 79291af64a83e59901de648ca2c81c8317235dc1 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemProducer.java 42ae00b3166b8adc8ad0ce102e1707d52946dc2e 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetMigrationMetaMessage.java PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/migration/MigrationPlan.java PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala 16732179f8f3783a62b1bb4f34ce46da44a3140e 
>   samza-core/src/main/scala/org/apache/samza/migration/JobRunnerMigration.scala PRE-CREATION 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamSystemFactory.java 84ae0b5e3d35c6cabf7f712610d2118c8982bd1b 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamWrappedConsumer.java 47a44b1df3f3abdc782ca1b29c520609265fd365 
>   samza-core/src/test/java/org/apache/samza/storage/TestStorageRecovery.java 52d450d79cc1f87fe43dcd9ef06dbed4432fa919 
>   samza-core/src/test/resources/test-migration-fail.properties PRE-CREATION 
>   samza-core/src/test/scala/org/apache/samza/job/TestJobRunner.scala 52057edbbcd08b7d6edde7e898466d26534f52f6 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointLogKey.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManager.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManagerFactory.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointMigration.scala PRE-CREATION 
>   samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemProducer.scala c0c34bf887d3c0bc4beb8a06a1897a82bba0c654 
>   samza-kafka/src/test/scala/old/checkpoint/TestKafkaCheckpointManager.scala PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38614/diff/
> 
> 
> Testing
> -------
> 
> ./bin/check-all.sh passed.
> 
> 
> Thanks,
> 
> Yi Pan (Data Infrastructure)
> 
>


Re: Review Request 38614: SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints

Posted by Navina Ramesh <nr...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38614/#review100355
-----------------------------------------------------------

Ship it!


lgtm! Let's get this checked-in :)


samza-core/src/test/scala/org/apache/samza/job/TestJobRunner.scala (line 54)
<https://reviews.apache.org/r/38614/#comment157537>

    This is weird. Is it a good idea to assert exception messages?


- Navina Ramesh


On Sept. 22, 2015, 11 p.m., Yi Pan (Data Infrastructure) wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38614/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2015, 11 p.m.)
> 
> 
> Review request for samza, Yan Fang and Navina Ramesh.
> 
> 
> Bugs: SAMZA-615
>     https://issues.apache.org/jira/browse/SAMZA-615
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints
> 
> 
> Diffs
> -----
> 
>   build.gradle 3a7fabcd4f4e5c8db950c3a33bba33618c5565b4 
>   checkstyle/import-control.xml bc07ae8116c00aecf2ac2fa3da58985c5ebe9e11 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemConsumer.java 79291af64a83e59901de648ca2c81c8317235dc1 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemProducer.java 42ae00b3166b8adc8ad0ce102e1707d52946dc2e 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetMigrationMetaMessage.java PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/migration/MigrationPlan.java PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala 16732179f8f3783a62b1bb4f34ce46da44a3140e 
>   samza-core/src/main/scala/org/apache/samza/migration/JobRunnerMigration.scala PRE-CREATION 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamSystemFactory.java 84ae0b5e3d35c6cabf7f712610d2118c8982bd1b 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamWrappedConsumer.java 47a44b1df3f3abdc782ca1b29c520609265fd365 
>   samza-core/src/test/java/org/apache/samza/storage/TestStorageRecovery.java 52d450d79cc1f87fe43dcd9ef06dbed4432fa919 
>   samza-core/src/test/resources/test-migration-fail.properties PRE-CREATION 
>   samza-core/src/test/scala/org/apache/samza/job/TestJobRunner.scala 52057edbbcd08b7d6edde7e898466d26534f52f6 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointLogKey.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManager.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManagerFactory.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointMigration.scala PRE-CREATION 
>   samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemProducer.scala c0c34bf887d3c0bc4beb8a06a1897a82bba0c654 
>   samza-kafka/src/test/scala/old/checkpoint/TestKafkaCheckpointManager.scala PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38614/diff/
> 
> 
> Testing
> -------
> 
> ./bin/check-all.sh passed.
> 
> 
> Thanks,
> 
> Yi Pan (Data Infrastructure)
> 
>


Re: Review Request 38614: SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints

Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38614/
-----------------------------------------------------------

(Updated Sept. 22, 2015, 11 p.m.)


Review request for samza, Yan Fang and Navina Ramesh.


Changes
-------

Fixed intermittent failure in TestJobRunner unit test


Bugs: SAMZA-615
    https://issues.apache.org/jira/browse/SAMZA-615


Repository: samza


Description
-------

SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints


Diffs (updated)
-----

  build.gradle 3a7fabcd4f4e5c8db950c3a33bba33618c5565b4 
  checkstyle/import-control.xml bc07ae8116c00aecf2ac2fa3da58985c5ebe9e11 
  samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemConsumer.java 79291af64a83e59901de648ca2c81c8317235dc1 
  samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemProducer.java 42ae00b3166b8adc8ad0ce102e1707d52946dc2e 
  samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetMigrationMetaMessage.java PRE-CREATION 
  samza-core/src/main/java/org/apache/samza/migration/MigrationPlan.java PRE-CREATION 
  samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala 16732179f8f3783a62b1bb4f34ce46da44a3140e 
  samza-core/src/main/scala/org/apache/samza/migration/JobRunnerMigration.scala PRE-CREATION 
  samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamSystemFactory.java 84ae0b5e3d35c6cabf7f712610d2118c8982bd1b 
  samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamWrappedConsumer.java 47a44b1df3f3abdc782ca1b29c520609265fd365 
  samza-core/src/test/java/org/apache/samza/storage/TestStorageRecovery.java 52d450d79cc1f87fe43dcd9ef06dbed4432fa919 
  samza-core/src/test/resources/test-migration-fail.properties PRE-CREATION 
  samza-core/src/test/scala/org/apache/samza/job/TestJobRunner.scala 52057edbbcd08b7d6edde7e898466d26534f52f6 
  samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointLogKey.scala PRE-CREATION 
  samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManager.scala PRE-CREATION 
  samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManagerFactory.scala PRE-CREATION 
  samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointMigration.scala PRE-CREATION 
  samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemProducer.scala c0c34bf887d3c0bc4beb8a06a1897a82bba0c654 
  samza-kafka/src/test/scala/old/checkpoint/TestKafkaCheckpointManager.scala PRE-CREATION 

Diff: https://reviews.apache.org/r/38614/diff/


Testing
-------

./bin/check-all.sh passed.


Thanks,

Yi Pan (Data Infrastructure)


Re: Review Request 38614: SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints

Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38614/
-----------------------------------------------------------

(Updated Sept. 22, 2015, 6:53 p.m.)


Review request for samza, Yan Fang and Navina Ramesh.


Changes
-------

Merged w/ SAMZA-731 changes and removed all write methods in the old KafkaCheckpointManager


Bugs: SAMZA-615
    https://issues.apache.org/jira/browse/SAMZA-615


Repository: samza


Description
-------

SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints


Diffs (updated)
-----

  build.gradle 3a7fabcd4f4e5c8db950c3a33bba33618c5565b4 
  checkstyle/import-control.xml bc07ae8116c00aecf2ac2fa3da58985c5ebe9e11 
  samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemConsumer.java 79291af64a83e59901de648ca2c81c8317235dc1 
  samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemProducer.java 42ae00b3166b8adc8ad0ce102e1707d52946dc2e 
  samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetMigrationMetaMessage.java PRE-CREATION 
  samza-core/src/main/java/org/apache/samza/migration/MigrationPlan.java PRE-CREATION 
  samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala 16732179f8f3783a62b1bb4f34ce46da44a3140e 
  samza-core/src/main/scala/org/apache/samza/migration/JobRunnerMigration.scala PRE-CREATION 
  samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamSystemFactory.java 84ae0b5e3d35c6cabf7f712610d2118c8982bd1b 
  samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamWrappedConsumer.java 47a44b1df3f3abdc782ca1b29c520609265fd365 
  samza-core/src/test/java/org/apache/samza/storage/TestStorageRecovery.java 52d450d79cc1f87fe43dcd9ef06dbed4432fa919 
  samza-core/src/test/resources/test-migration-fail.properties PRE-CREATION 
  samza-core/src/test/scala/org/apache/samza/job/TestJobRunner.scala 52057edbbcd08b7d6edde7e898466d26534f52f6 
  samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointLogKey.scala PRE-CREATION 
  samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManager.scala PRE-CREATION 
  samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManagerFactory.scala PRE-CREATION 
  samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointMigration.scala PRE-CREATION 
  samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemProducer.scala c0c34bf887d3c0bc4beb8a06a1897a82bba0c654 
  samza-kafka/src/test/scala/old/checkpoint/TestKafkaCheckpointManager.scala PRE-CREATION 

Diff: https://reviews.apache.org/r/38614/diff/


Testing (updated)
-------

./bin/check-all.sh passed.


Thanks,

Yi Pan (Data Infrastructure)


Re: Review Request 38614: SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints

Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.

> On Sept. 22, 2015, 10 a.m., Navina Ramesh wrote:
> > samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamSystemFactory.java, line 26
> > <https://reviews.apache.org/r/38614/diff/1/?file=1080053#file1080053line26>
> >
> >     Can you please import only the necessary classes?

Sure. Had a note but missed it in the final cleanup. Will do.


- Yi


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


On Sept. 22, 2015, 8:20 a.m., Yi Pan (Data Infrastructure) wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38614/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2015, 8:20 a.m.)
> 
> 
> Review request for samza, Yan Fang and Navina Ramesh.
> 
> 
> Bugs: SAMZA-615
>     https://issues.apache.org/jira/browse/SAMZA-615
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints
> 
> 
> Diffs
> -----
> 
>   build.gradle 3a7fabcd4f4e5c8db950c3a33bba33618c5565b4 
>   checkstyle/import-control.xml bc07ae8116c00aecf2ac2fa3da58985c5ebe9e11 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemConsumer.java 79291af64a83e59901de648ca2c81c8317235dc1 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemProducer.java 42ae00b3166b8adc8ad0ce102e1707d52946dc2e 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/CoordinatorStreamMessage.java 68e50a12eb0c7d5d66a11da48cc28c5b2dfa7b87 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetMigrationMetaMessage.java PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/migration/MigrationPlan.java PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala 16732179f8f3783a62b1bb4f34ce46da44a3140e 
>   samza-core/src/main/scala/org/apache/samza/migration/JobRunnerMigration.scala PRE-CREATION 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamSystemFactory.java 84ae0b5e3d35c6cabf7f712610d2118c8982bd1b 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamWrappedConsumer.java 47a44b1df3f3abdc782ca1b29c520609265fd365 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamMessage.java 60e2e5df65db5435c47ea525d85dee451279c798 
>   samza-core/src/test/java/org/apache/samza/storage/TestStorageRecovery.java 52d450d79cc1f87fe43dcd9ef06dbed4432fa919 
>   samza-core/src/test/resources/test-migration-fail.properties PRE-CREATION 
>   samza-core/src/test/scala/org/apache/samza/job/TestJobRunner.scala 52057edbbcd08b7d6edde7e898466d26534f52f6 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointLogKey.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManager.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManagerFactory.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointMigration.scala PRE-CREATION 
>   samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemProducer.scala c0c34bf887d3c0bc4beb8a06a1897a82bba0c654 
>   samza-kafka/src/test/scala/old/checkpoint/TestKafkaCheckpointManager.scala PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38614/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Yi Pan (Data Infrastructure)
> 
>


Re: Review Request 38614: SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints

Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.

> On Sept. 22, 2015, 10 a.m., Navina Ramesh wrote:
> > samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/CoordinatorStreamMessage.java, line 295
> > <https://reviews.apache.org/r/38614/diff/1/?file=1080048#file1080048line295>
> >
> >     I separated out this change for SAMZA-731 :) https://reviews.apache.org/r/38395/

OK. I will check SAMZA-731 patch and commit that first then. Thanks!


- Yi


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


On Sept. 22, 2015, 8:20 a.m., Yi Pan (Data Infrastructure) wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38614/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2015, 8:20 a.m.)
> 
> 
> Review request for samza, Yan Fang and Navina Ramesh.
> 
> 
> Bugs: SAMZA-615
>     https://issues.apache.org/jira/browse/SAMZA-615
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints
> 
> 
> Diffs
> -----
> 
>   build.gradle 3a7fabcd4f4e5c8db950c3a33bba33618c5565b4 
>   checkstyle/import-control.xml bc07ae8116c00aecf2ac2fa3da58985c5ebe9e11 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemConsumer.java 79291af64a83e59901de648ca2c81c8317235dc1 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemProducer.java 42ae00b3166b8adc8ad0ce102e1707d52946dc2e 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/CoordinatorStreamMessage.java 68e50a12eb0c7d5d66a11da48cc28c5b2dfa7b87 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetMigrationMetaMessage.java PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/migration/MigrationPlan.java PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala 16732179f8f3783a62b1bb4f34ce46da44a3140e 
>   samza-core/src/main/scala/org/apache/samza/migration/JobRunnerMigration.scala PRE-CREATION 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamSystemFactory.java 84ae0b5e3d35c6cabf7f712610d2118c8982bd1b 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamWrappedConsumer.java 47a44b1df3f3abdc782ca1b29c520609265fd365 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamMessage.java 60e2e5df65db5435c47ea525d85dee451279c798 
>   samza-core/src/test/java/org/apache/samza/storage/TestStorageRecovery.java 52d450d79cc1f87fe43dcd9ef06dbed4432fa919 
>   samza-core/src/test/resources/test-migration-fail.properties PRE-CREATION 
>   samza-core/src/test/scala/org/apache/samza/job/TestJobRunner.scala 52057edbbcd08b7d6edde7e898466d26534f52f6 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointLogKey.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManager.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManagerFactory.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointMigration.scala PRE-CREATION 
>   samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemProducer.scala c0c34bf887d3c0bc4beb8a06a1897a82bba0c654 
>   samza-kafka/src/test/scala/old/checkpoint/TestKafkaCheckpointManager.scala PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38614/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Yi Pan (Data Infrastructure)
> 
>


Re: Review Request 38614: SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints

Posted by Navina Ramesh <nr...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38614/#review99963
-----------------------------------------------------------



samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/CoordinatorStreamMessage.java (line 295)
<https://reviews.apache.org/r/38614/#comment157011>

    I separated out this change for SAMZA-731 :) https://reviews.apache.org/r/38395/



samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamSystemFactory.java (line 26)
<https://reviews.apache.org/r/38614/#comment157012>

    Can you please import only the necessary classes?



samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamMessage.java (line 74)
<https://reviews.apache.org/r/38614/#comment157014>

    Same comment as above. I separated out these changes in SAMZA-731 here - https://reviews.apache.org/r/38395/ :)



samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManager.scala (line 331)
<https://reviews.apache.org/r/38614/#comment157019>

    Awesome! I tried removing start and stop methods in KCM and everything went for a toss. This looks good :)


- Navina Ramesh


On Sept. 22, 2015, 8:20 a.m., Yi Pan (Data Infrastructure) wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38614/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2015, 8:20 a.m.)
> 
> 
> Review request for samza, Yan Fang and Navina Ramesh.
> 
> 
> Bugs: SAMZA-615
>     https://issues.apache.org/jira/browse/SAMZA-615
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-615: Add auto-checkpoint migration for Kafka checkpoints
> 
> 
> Diffs
> -----
> 
>   build.gradle 3a7fabcd4f4e5c8db950c3a33bba33618c5565b4 
>   checkstyle/import-control.xml bc07ae8116c00aecf2ac2fa3da58985c5ebe9e11 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemConsumer.java 79291af64a83e59901de648ca2c81c8317235dc1 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemProducer.java 42ae00b3166b8adc8ad0ce102e1707d52946dc2e 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/CoordinatorStreamMessage.java 68e50a12eb0c7d5d66a11da48cc28c5b2dfa7b87 
>   samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetMigrationMetaMessage.java PRE-CREATION 
>   samza-core/src/main/java/org/apache/samza/migration/MigrationPlan.java PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala 16732179f8f3783a62b1bb4f34ce46da44a3140e 
>   samza-core/src/main/scala/org/apache/samza/migration/JobRunnerMigration.scala PRE-CREATION 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamSystemFactory.java 84ae0b5e3d35c6cabf7f712610d2118c8982bd1b 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamWrappedConsumer.java 47a44b1df3f3abdc782ca1b29c520609265fd365 
>   samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamMessage.java 60e2e5df65db5435c47ea525d85dee451279c798 
>   samza-core/src/test/java/org/apache/samza/storage/TestStorageRecovery.java 52d450d79cc1f87fe43dcd9ef06dbed4432fa919 
>   samza-core/src/test/resources/test-migration-fail.properties PRE-CREATION 
>   samza-core/src/test/scala/org/apache/samza/job/TestJobRunner.scala 52057edbbcd08b7d6edde7e898466d26534f52f6 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointLogKey.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManager.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointManagerFactory.scala PRE-CREATION 
>   samza-kafka/src/main/scala/old/checkpoint/KafkaCheckpointMigration.scala PRE-CREATION 
>   samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemProducer.scala c0c34bf887d3c0bc4beb8a06a1897a82bba0c654 
>   samza-kafka/src/test/scala/old/checkpoint/TestKafkaCheckpointManager.scala PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38614/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Yi Pan (Data Infrastructure)
> 
>