samza-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Naveen Somasundaram" <navg...@gmail.com>
Subject Re: Review Request 29012: SAMZA-226 Autocreate changelog topic
Date Mon, 15 Dec 2014 23:46:56 GMT


> On Dec. 15, 2014, 5:31 p.m., Chris Riccomini wrote:
> > samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala, line
99
> > <https://reviews.apache.org/r/29012/diff/1/?file=790800#file790800line99>
> >
> >     Nit: double space newline.

done


> On Dec. 15, 2014, 5:31 p.m., Chris Riccomini wrote:
> > samza-kafka/src/main/scala/org/apache/samza/config/KafkaConfig.scala, line 46
> > <https://reviews.apache.org/r/29012/diff/1/?file=790805#file790805line46>
> >
> >     Shouldn't this have a trailing '.' after it? You do config.subset with it. As
it stands now, I think you'd have to set your config to:
> >     
> >     stores.mystore.changelog.kafkacleanup.policy=compa
> >     ct
> >     stores.mystore.changelog.kafkasegment.bytes=1234
> >     
> >     It's better if it's:
> >     
> >     stores.mystore.changelog.kafka.cleanup.policy
> >     stores.mystore.changelog.kafka.segment.bytes

good catch! That's what I intended to do.


> On Dec. 15, 2014, 5:31 p.m., Chris Riccomini wrote:
> > samza-kafka/src/main/scala/org/apache/samza/config/KafkaConfig.scala, line 106
> > <https://reviews.apache.org/r/29012/diff/1/?file=790805#file790805line106>
> >
> >     Nit: This name is a little misleading. It only returns changelog enabld Kakfa
stores (not all stores with a changelog enabled).

I was thinking about it when I wrote it, since it was in KafkaConfig, I thought it's OK. But
I guess, there is no harm in using "getKafkaChangelogEnabledStores()"


> On Dec. 15, 2014, 5:31 p.m., Chris Riccomini wrote:
> > samza-kafka/src/main/scala/org/apache/samza/config/KafkaConfig.scala, line 112
> > <https://reviews.apache.org/r/29012/diff/1/?file=790805#file790805line112>
> >
> >     Use getSystemFactory() here.

done!


> On Dec. 15, 2014, 5:31 p.m., Chris Riccomini wrote:
> > samza-kafka/src/main/scala/org/apache/samza/config/KafkaConfig.scala, line 116
> > <https://reviews.apache.org/r/29012/diff/1/?file=790805#file790805line116>
> >
> >     SamzaException message should say which SystemStream/changelogConfig failed
to match.

done


- Naveen


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


On Dec. 12, 2014, 10:42 p.m., Naveen Somasundaram wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29012/
> -----------------------------------------------------------
> 
> (Updated Dec. 12, 2014, 10:42 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-226 Autocreate changelog topic
> 
> 
> Diffs
> -----
> 
>   docs/learn/documentation/versioned/jobs/configuration-table.html 6a5d209c6e10067c25a38c20494a5ad69025bd3b

>   samza-api/src/main/java/org/apache/samza/config/Config.java 2048e90e80e21086eb59be57f3bcd5ebf92b2811

>   samza-api/src/main/java/org/apache/samza/system/SystemAdmin.java 571c60631357ea9a0b4fa24e7253008619ef2f32

>   samza-api/src/main/java/org/apache/samza/util/SinglePartitionWithoutOffsetsSystemAdmin.java
38e313f3c39454110efd354e6ca025869fa930cd 
>   samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala 2f1568d842dc4d3f489e6ecc4e5178a59252feee

>   samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala b8719c36c2b9346bcd3f291e23b33d2c00cebfa9

>   samza-core/src/main/scala/org/apache/samza/system/filereader/FileReaderSystemAdmin.scala
98e92bc12f3e2827cdec02f1ce94d7e2314e4b4e 
>   samza-core/src/test/scala/org/apache/samza/checkpoint/TestOffsetManager.scala 35e7f6bfb9fe5a964d397d168b00bc1fd859da7c

>   samza-core/src/test/scala/org/apache/samza/coordinator/TestJobCoordinator.scala 1eb0edab1bc792ccf8c503b03687284451ab0f34

>   samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManagerFactory.scala
f7db2a1fc5217343bf808e3dcf20315822c1bcde 
>   samza-kafka/src/main/scala/org/apache/samza/config/KafkaConfig.scala 9fc1f56d4404ec7722c0d34fde2804e981b41309

>   samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemAdmin.scala 5ac33ea36da451250655d9dd373692b964322b41

>   samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemFactory.scala 4ed5e881031e019d8df6de259cabb658820a3ba0

>   samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala d660b91fb7a1029a47d5e083759b8971ad97e617

>   samza-kafka/src/test/scala/org/apache/samza/system/kafka/TestKafkaSystemAdmin.scala
5ceb1093a66cb57e298d4b3ccdd24845dbb41b58 
>   samza-test/src/main/java/org/apache/samza/system/mock/MockSystemAdmin.java fa1d51b290013a3913d64884dc43907a76670849

>   samza-test/src/test/scala/org/apache/samza/test/integration/TestStatefulTask.scala
118f5eee22016db3b802c32fb26c5d72fa61f1a7 
>   samza-yarn/src/test/scala/org/apache/samza/job/yarn/TestSamzaAppMasterTaskManager.scala
cab5101c5c9e2a979bca545fa8046e93dcfe46e2 
> 
> Diff: https://reviews.apache.org/r/29012/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Naveen Somasundaram
> 
>


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