[Discuss] Samza 0.13.1 release
Hi all, There have been some new featuresand critical bug fixes added to master since 0.13.0 release, which makes SamzaStandalone features more stable. It is now good enough to warrant a new minorrelease. We will continue to test for stability and performance in the next fewweeks. Here are the main JIRA tickets that will be included in this release (but notlimited to):SAMZA-1165: Cleanup datacreated by ZkStandalone in ZKSAMZA-1324: Add ametricsreporter lifecycle for JobCoordinator component of StreamProcessorSAMZA-1336: Standalone sessionexpiration propagationSAMZA-1337: LocalApplicationRunner needs to support StreamTaskSAMZA-1339: Add standaloneintegration tests… There are also quite a few bug fixes in 0.13.1, please check the complete list of changes in 0.13.1 here. Most JIRAs in the list havebeen completed and merged, with the following one remaining, but we should tryto get it completed before 0.13.1 is released.SAMZA-1385: Coordinationutils in LocalApplicationRunner uses same Zk node as ZkJobCoordinatorFactoryfor leader election Here's what I propose:1. Cut an 0.13.1 release branch.2. Work on getting the remaining open JIRA done.3. Target a release vote by Aug 18. Thoughts? Fred
Re: Review Request 53297: Initial version of adding metrics into samza rest.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/#review155753 --- Ship it! Ship It! - Fred Ji On Nov. 11, 2016, 12:22 a.m., Shanthoosh Venkataraman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53297/ > --- > > (Updated Nov. 11, 2016, 12:22 a.m.) > > > Review request for samza and Jake Maes. > > > Repository: samza > > > Description > --- > > This patch aims at enabling users to define custom reporters to send metrics > from the monitors. Configurations required for the definition of the metrics > reporters follows the same convention as of the samza jobs. > > > Diffs > - > > docs/learn/documentation/versioned/rest/monitors.md > 46678bbe5fed99f767c3324dc9578ee1a64cec66 > samza-core/src/main/java/org/apache/samza/util/MetricsReporterLoader.java > PRE-CREATION > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > e0468ee89c89fd720834461771ebb36475475bcb > > samza-core/src/main/scala/org/apache/samza/metrics/ContainerProcessManagerMetrics.scala > f24beb1e099dd44b15b475e0a4a7f70560c6965e > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java > 2a3e83a24a5343bb53b93fc9d0a647c1b253714b > samza-rest/src/test/java/org/apache/samza/rest/TestSamzaRestService.java > PRE-CREATION > > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala > 8a5b4aaea6e11a5af999f12d50e5b6135dbc70ca > > Diff: https://reviews.apache.org/r/53297/diff/ > > > Testing > --- > > Unit tests are done to verify the intended functionality. > > > Thanks, > > Shanthoosh Venkataraman > >
Re: Review Request 53326: SAMZA-1045 Move classes from samza-operator/api into samza-api.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53326/#review154458 --- Ship it! Ship It! - Fred Ji On Nov. 1, 2016, 6:40 a.m., Jagadish Venkatraman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53326/ > --- > > (Updated Nov. 1, 2016, 6:40 a.m.) > > > Review request for samza, Jake Maes, Yi Pan (Data Infrastructure), Prateek > Maheshwari, and Xinyu Liu. > > > Repository: samza > > > Description > --- > > Refactor API classes from Samza-Operator/operator into samza-api. > > - Programmers write code against classes on samza-api, and it would be nice > to not have a dependency on samza-operator (when defining the transform logic > for the operator). > - For split-deployment to be effective, it'd be nice to independently evolve > samza-operator components. > - Fixed checkstyle issues and formating. > > Note: There are a couple of bugs that I found in the operator logic > (SAMZA-914/915). They are intentionally kept out of the scope this RB > > > Diffs > - > > build.gradle aeefd1cfd049efa37d20e12e34232600ae6bd127 > checkstyle/checkstyle.xml 770b5e7f7a091198bbf53b3908600f9ac0caa4c7 > gradle.properties 16e1f5d43f0415c511689480f8cb67d84e2baadf > gradle/wrapper/gradle-wrapper.jar a7634b071cb255e91a4572934e55b8cd8877b3e4 > gradle/wrapper/gradle-wrapper.properties > 78596c0ebdd585e2d674cbcec930d0a8a2a08e74 > gradlew 91a7e269e19dfc62e27137a0b57ef3e430cee4fd > samza-api/src/main/java/org/apache/samza/storage/StorageEngineFactory.java > 963ccf2b6222f0fee00705923d921f91ed481fbc > > samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamWrappedConsumer.java > 429573b480112c7491303dc410d78f37a308c4a7 > > samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamSystemConsumer.java > 0e73e18bd55e343e1a5122be7e8f3c666b797dc5 > > samza-operator/src/main/java/org/apache/samza/operators/api/MessageStream.java > b5e1028662a67e6248722ef7c842c565fef7a458 > > samza-operator/src/main/java/org/apache/samza/operators/api/MessageStreams.java > 59dd91c2f537828640b1ede6b6366e37f6b5c63b > > samza-operator/src/main/java/org/apache/samza/operators/api/TriggerBuilder.java > fc3ea37563ded2fcceeb332b0edfef45208bb9bb > > samza-operator/src/main/java/org/apache/samza/operators/api/WindowState.java > 402cc42f63bad0c8745c6a480b03769f92053622 > samza-operator/src/main/java/org/apache/samza/operators/api/Windows.java > e557b34c25ff425385aedae24bad6bb89ee03a30 > > samza-operator/src/main/java/org/apache/samza/operators/api/data/IncomingSystemMessage.java > ba74618961d3ea53ad455de3263c59c0ca2a0fa7 > > samza-operator/src/main/java/org/apache/samza/operators/api/data/InputSystemMessage.java > c7860254e38a897e42e00b60111526db0f7510ea > > samza-operator/src/main/java/org/apache/samza/operators/api/data/LongOffset.java > f059b337e299aab07c85af9866371c87fcf59786 > > samza-operator/src/main/java/org/apache/samza/operators/api/data/Message.java > 9b53b4582ed12fa7d9948ff756ed9d0a4b38e280 > > samza-operator/src/main/java/org/apache/samza/operators/api/data/Offset.java > 0fac2c0123b133566630bdf11e9b1c00a207bd2e > > samza-operator/src/main/java/org/apache/samza/operators/api/internal/Operators.java > e9bfe0b38c1048ecc7fe634ed7b0d71fee6f2ac6 > > samza-operator/src/main/java/org/apache/samza/operators/api/internal/Trigger.java > 33a0134e88b966d8f08658f18ca73564702305fa > > samza-operator/src/main/java/org/apache/samza/operators/api/internal/WindowFn.java > 1fd88e76119960d1e6b83189bb571e64ac329489 > > samza-operator/src/main/java/org/apache/samza/operators/api/internal/WindowOutput.java > e202c20cb0af6e53268e0c80820bfb1d7fc80892 > > samza-operator/src/main/java/org/apache/samza/operators/impl/ChainedOperators.java > 59de16bd11fee34b23baac14d1d32cf9df555058 > > samza-operator/src/main/java/org/apache/samza/operators/impl/OperatorFactory.java > f16cbc6bf405851488c22b943a2256864b7a9f07 > > samza-operator/src/main/java/org/apache/samza/operators/impl/OperatorImpl.java > 3ca8bdeb7d73c72f469fa33a152ed7518f9e8ac4 > > samza-operator/src/main/java/org/apache/samza/operators/impl/ProcessorContext.java > 5a375bc7bb3b00fd252c6b1bbe211de8feddd942 > > samza-operator/src/main/java/org/apache/samza/operators/impl/SimpleOperatorImpl.j
Re: Review Request 53326: SAMZA-1045 Move classes from samza-operator/api into samza-api.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53326/#review154347 --- build.gradle (line 126) <https://reviews.apache.org/r/53326/#comment223863> I would recommend keeping the checkstyle on and fixing the incompatibility issue so that checkstyle plugin can caputre the style issue in the build process. If we disable it, some of the style issues (although minor) may be hidden and be merged into master. - Fred Ji On Oct. 31, 2016, 11:27 p.m., Jagadish Venkatraman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53326/ > --- > > (Updated Oct. 31, 2016, 11:27 p.m.) > > > Review request for samza, Jake Maes, Yi Pan (Data Infrastructure), Prateek > Maheshwari, and Xinyu Liu. > > > Repository: samza > > > Description > --- > > Refactor API classes from Samza-Operator/operator into samza-api. > > - Programmers write code against classes on samza-api, and it would be nice > to not have a dependency on samza-operator (when defining the transform logic > for the operator). > - For split-deployment to be effective, it'd be nice to independently evolve > samza-operator components. > > Note: There are a couple of bugs that I found in the operator logic > (SAMZA-914/915). They are intentionally kept out of the scope this RB > > > Diffs > - > > build.gradle aeefd1cfd049efa37d20e12e34232600ae6bd127 > checkstyle/checkstyle.xml 770b5e7f7a091198bbf53b3908600f9ac0caa4c7 > gradle.properties 16e1f5d43f0415c511689480f8cb67d84e2baadf > > samza-operator/src/main/java/org/apache/samza/operators/api/MessageStream.java > b5e1028662a67e6248722ef7c842c565fef7a458 > > samza-operator/src/main/java/org/apache/samza/operators/api/MessageStreams.java > 59dd91c2f537828640b1ede6b6366e37f6b5c63b > > samza-operator/src/main/java/org/apache/samza/operators/api/TriggerBuilder.java > fc3ea37563ded2fcceeb332b0edfef45208bb9bb > > samza-operator/src/main/java/org/apache/samza/operators/api/WindowState.java > 402cc42f63bad0c8745c6a480b03769f92053622 > samza-operator/src/main/java/org/apache/samza/operators/api/Windows.java > e557b34c25ff425385aedae24bad6bb89ee03a30 > > samza-operator/src/main/java/org/apache/samza/operators/api/data/IncomingSystemMessage.java > ba74618961d3ea53ad455de3263c59c0ca2a0fa7 > > samza-operator/src/main/java/org/apache/samza/operators/api/data/InputSystemMessage.java > c7860254e38a897e42e00b60111526db0f7510ea > > samza-operator/src/main/java/org/apache/samza/operators/api/data/LongOffset.java > f059b337e299aab07c85af9866371c87fcf59786 > > samza-operator/src/main/java/org/apache/samza/operators/api/data/Message.java > 9b53b4582ed12fa7d9948ff756ed9d0a4b38e280 > > samza-operator/src/main/java/org/apache/samza/operators/api/data/Offset.java > 0fac2c0123b133566630bdf11e9b1c00a207bd2e > > samza-operator/src/main/java/org/apache/samza/operators/api/internal/Operators.java > e9bfe0b38c1048ecc7fe634ed7b0d71fee6f2ac6 > > samza-operator/src/main/java/org/apache/samza/operators/api/internal/Trigger.java > 33a0134e88b966d8f08658f18ca73564702305fa > > samza-operator/src/main/java/org/apache/samza/operators/api/internal/WindowFn.java > 1fd88e76119960d1e6b83189bb571e64ac329489 > > samza-operator/src/main/java/org/apache/samza/operators/api/internal/WindowOutput.java > e202c20cb0af6e53268e0c80820bfb1d7fc80892 > > samza-operator/src/main/java/org/apache/samza/operators/impl/ChainedOperators.java > 59de16bd11fee34b23baac14d1d32cf9df555058 > > samza-operator/src/main/java/org/apache/samza/operators/impl/OperatorFactory.java > f16cbc6bf405851488c22b943a2256864b7a9f07 > > samza-operator/src/main/java/org/apache/samza/operators/impl/OperatorImpl.java > 3ca8bdeb7d73c72f469fa33a152ed7518f9e8ac4 > > samza-operator/src/main/java/org/apache/samza/operators/impl/ProcessorContext.java > 5a375bc7bb3b00fd252c6b1bbe211de8feddd942 > > samza-operator/src/main/java/org/apache/samza/operators/impl/SimpleOperatorImpl.java > b29d9c8c412c70820ac8355137df447d373af1f6 > > samza-operator/src/main/java/org/apache/samza/operators/impl/SinkOperatorImpl.java > 5d25cfa106424f45660a45434c4abffd242b8672 > > samza-operator/src/main/java/org/apache/samza/operators/impl/StateStoreImpl.java > f573fd04e1aaf827d2ca52a5edca8f7060723aa0 > > samza-operator/src/main/java/org/apache/samza/operators/
Re: Review Request 53002: Do not load the monitor, if the MonitorFactoryClass is not defined for the monitor in the config.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53002/#review153208 --- Ship it! samza-rest/src/main/java/org/apache/samza/monitor/SamzaMonitorService.java (line 71) <https://reviews.apache.org/r/53002/#comment222498> nit: this line of comment is not needed since log warn is very clear. - Fred Ji On Oct. 18, 2016, 10:53 p.m., Shanthoosh Venkataraman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53002/ > --- > > (Updated Oct. 18, 2016, 10:53 p.m.) > > > Review request for samza and Jake Maes. > > > Repository: samza > > > Description > --- > > This patch aims to not load the monitors if the monitor factory class is not > defined(set) for the monitor in the config. This will enable the users to > turn on/off the monitors in samza-rest easily(just by setting the > monitorFactoryClass config associated with monitor a to empty string.) > > > Diffs > - > > samza-rest/src/main/java/org/apache/samza/monitor/SamzaMonitorService.java > ce947f7ae1175acc1ee9aa75991c726848072694 > samza-rest/src/test/java/org/apache/samza/monitor/TestMonitorService.java > 4618b54f5af861383df45bf7185622d36d17cd5e > > samza-rest/src/test/java/org/apache/samza/monitor/mock/MockMonitorFactory.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/53002/diff/ > > > Testing > --- > > Unit testing and manual testing are done to verify the functionality. > > > Thanks, > > Shanthoosh Venkataraman > >
Re: Review Request 50619: SAMZA-963: add KV storage engine timers to help identify the issues on kv stores and also add unit test
> On Aug. 5, 2016, 6:31 p.m., Yi Pan (Data Infrastructure) wrote: > > samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala, > > line 135 > > <https://reviews.apache.org/r/50619/diff/1/?file=1458229#file1458229line135> > > > > Why do we need to add the default function, if you already defined the > > default in KeyValueStorageEngine.scala? Thanks a lot. My initial consideration was to minimize potential errors if somebody would like to add new paramters for this constructor since there are a lot already. I also saw some similar uses in the existing samza code base as well. I am OK on either one. I can follow whatever code standard our team has. - Fred --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50619/#review144968 ------- On July 29, 2016, 10:24 p.m., Fred Ji wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50619/ > --- > > (Updated July 29, 2016, 10:24 p.m.) > > > Review request for samza, Chris Pettitt, Jake Maes, and Yi Pan (Data > Infrastructure). > > > Bugs: SAMZA-963 > https://issues.apache.org/jira/browse/SAMZA-963 > > > Repository: samza > > > Description > --- > > SAMZA-963: add KV storage engine timers to help identify the issues on kv > stores and also add unit test > > > Diffs > - > > > samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala > c975893a42689732c39c39600fecacee843bf9d6 > > samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala > a3ffc421020b7a84c40b2101f2e37db8a20690cb > > samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineMetrics.scala > 233fba91caf041bfb78189efef00ce8fc56f9f15 > > samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStorageEngine.scala > PRE-CREATION > > Diff: https://reviews.apache.org/r/50619/diff/ > > > Testing > --- > > 1. unit test is successful on a newly created test file for > KeyValueStorageEngine: ./gradlew clean :samza-kv_2.10:test > -Dtest.single=TestKeyValueStorageEngine > 2. build and all unit tests are successful: ./gradlew clean build > 3. ./gradlew checkstyleMain checkstyleTest passed > 4. manually tested on local machine for a stateful sample job depending on > KVStore, and from jconsole, the corresponding metrics were seen in mbeans > (see attached file) and the metrics were updated as expected. > > > File Attachments > > > snapshot of KeyValueStorageEngineMetrics bean from jconsole on local test > > https://reviews.apache.org/media/uploaded/files/2016/07/29/ce4b0456-73f9-44d5-af7d-0c45bf91bcaa__KeyValueStorageEngineMetricsFromJconsole.png > > > Thanks, > > Fred Ji > >
Re: Review Request 50682: SAMZA-993 Fix logging bug for some scala versions
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50682/#review144420 --- Ship it! Ship It! - Fred Ji On Aug. 2, 2016, 12:13 a.m., Jake Maes wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50682/ > --- > > (Updated Aug. 2, 2016, 12:13 a.m.) > > > Review request for samza, Boris Shkolnik, Chris Pettitt, Fred Ji, Jake Maes, > Navina Ramesh, Jagadish Venkatraman, Xinyu Liu, and Yi Pan (Data > Infrastructure). > > > Bugs: SAMZA-993 > https://issues.apache.org/jira/browse/SAMZA-993 > > > Repository: samza > > > Description > --- > > SAMZA-993 Fix logging bug for some scala versions > > > Diffs > - > > samza-core/src/main/scala/org/apache/samza/util/Logging.scala > 9a4ed897d58e90d61fdde14dd51ac5ba2d9edd36 > > Diff: https://reviews.apache.org/r/50682/diff/ > > > Testing > --- > > Running checkall now... > > > Thanks, > > Jake Maes > >
Re: Review Request 50527: SAMZA-970: fix integration tests
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50527/#review144228 --- Ship it! Ship It! - Fred Ji On July 29, 2016, 5:22 p.m., Xinyu Liu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50527/ > --- > > (Updated July 29, 2016, 5:22 p.m.) > > > Review request for samza and Navina Ramesh. > > > Repository: samza > > > Description > --- > > Upgrade python virturalenv version and ping down the zopkio version. > > > Diffs > - > > bin/integration-tests.sh af00b5f41e0f521bcef1cf94779ea67c434db469 > samza-test/src/main/python/configs/tests.json > 30be820d1f8f27629ab15f853533d558af8b53a5 > samza-test/src/main/python/requirements.txt > 2ae95908248516b5b26e671f24fa680f7b801675 > samza-test/src/main/python/samza_job_yarn_deployer.py > 38635ca5899c43fb61d6b4042e8543f0508fd41b > > Diff: https://reviews.apache.org/r/50527/diff/ > > > Testing > --- > > run integration tests and now they work fine. > > > Thanks, > > Xinyu Liu > >
Re: Review Request 50590: Update jackson version in hello-samza to match samza
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50590/#review144226 --- Ship it! Ship It! - Fred Ji On July 29, 2016, 12:34 a.m., Xinyu Liu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50590/ > --- > > (Updated July 29, 2016, 12:34 a.m.) > > > Review request for samza, Jake Maes, Navina Ramesh, and Yi Pan (Data > Infrastructure). > > > Repository: samza-hello-samza > > > Description > --- > > hello-samza has an outdated version of jackson lib version which causes lib > incompatibility issues (1.8.5 vs. 1.9.13). Update the jackson version to fix > it. > > > Diffs > - > > pom.xml 4bec5e06e1244455a4092d552220f0be416a4cca > > Diff: https://reviews.apache.org/r/50590/diff/ > > > Testing > --- > > Tested by running hello-world and check the output. > > > Thanks, > > Xinyu Liu > >
Re: Review Request 50614: SAMZA-970 - Problems with integration tests and SAMZA-987 - Preparing for 0.10.1 version release
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50614/#review144225 --- docs/_config.yml (line 26) <https://reviews.apache.org/r/50614/#comment210226> [Info] why not keep it as latest? - Fred Ji On July 29, 2016, 9:37 p.m., Navina Ramesh wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50614/ > --- > > (Updated July 29, 2016, 9:37 p.m.) > > > Review request for samza, Jake Maes, Xinyu Liu, and Yi Pan (Data > Infrastructure). > > > Bugs: SAMZA-987 > https://issues.apache.org/jira/browse/SAMZA-987 > > > Repository: samza > > > Description > --- > > SAMZA-987 - Preparing for 0.10.1 version release > > > Diffs > - > > bin/integration-tests.sh af00b5f41e0f521bcef1cf94779ea67c434db469 > docs/_config.yml dc1a66fa743d464c70d92406540fd7122c45272c > docs/startup/hello-samza/versioned/index.md > e1ef4da845fdc7ecb42456b834a112986c13a5d5 > gradle.properties 16e1f5d43f0415c511689480f8cb67d84e2baadf > samza-test/src/main/python/configs/tests.json > 30be820d1f8f27629ab15f853533d558af8b53a5 > samza-test/src/main/python/requirements.txt > 2ae95908248516b5b26e671f24fa680f7b801675 > samza-test/src/main/python/samza_job_yarn_deployer.py > 38635ca5899c43fb61d6b4042e8543f0508fd41b > > Diff: https://reviews.apache.org/r/50614/diff/ > > > Testing > --- > > ./bin/check-all.sh > > > Thanks, > > Navina Ramesh > >
Review Request 50619: SAMZA-963: add KV storage engine timers to help identify the issues on kv stores and also add unit test
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50619/ --- Review request for samza, Chris Pettitt, Jake Maes, and Yi Pan (Data Infrastructure). Bugs: SAMZA-963 https://issues.apache.org/jira/browse/SAMZA-963 Repository: samza Description --- SAMZA-963: add KV storage engine timers to help identify the issues on kv stores and also add unit test Diffs - samza-kv/src/main/scala/org/apache/samza/storage/kv/BaseKeyValueStorageEngineFactory.scala c975893a42689732c39c39600fecacee843bf9d6 samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngine.scala a3ffc421020b7a84c40b2101f2e37db8a20690cb samza-kv/src/main/scala/org/apache/samza/storage/kv/KeyValueStorageEngineMetrics.scala 233fba91caf041bfb78189efef00ce8fc56f9f15 samza-kv/src/test/scala/org/apache/samza/storage/kv/TestKeyValueStorageEngine.scala PRE-CREATION Diff: https://reviews.apache.org/r/50619/diff/ Testing --- 1. unit test is successful on a newly created test file for KeyValueStorageEngine: ./gradlew clean :samza-kv_2.10:test -Dtest.single=TestKeyValueStorageEngine 2. build and all unit tests are successful: ./gradlew clean build 3. ./gradlew checkstyleMain checkstyleTest passed 4. manually tested on local machine for a stateful sample job depending on KVStore, and from jconsole, the corresponding metrics were seen in mbeans (see attached file) and the metrics were updated as expected. File Attachments snapshot of KeyValueStorageEngineMetrics bean from jconsole on local test https://reviews.apache.org/media/uploaded/files/2016/07/29/ce4b0456-73f9-44d5-af7d-0c45bf91bcaa__KeyValueStorageEngineMetricsFromJconsole.png Thanks, Fred Ji
Re: Review Request 50317: SAMZA-978: update md files to resolve inconsistent links, broken links and some confusing sentences
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50317/ --- (Updated July 22, 2016, 11:23 p.m.) Review request for samza, Jon Bringhurst, Jagadish Venkatraman, and Yi Pan (Data Infrastructure). Changes --- fixed the Bug link for the RB information. The bugs field just requires the bug number but not the whole link. Bugs: SAMZA-978 https://issues.apache.org/jira/browse/SAMZA-978 Repository: samza Description --- 1. resolve the inconsistent next section link at the end of each section on samza.apache.org/learn/documentation/0.10/; 2. resolove some broken links such as Coda Hale's metrics library and Linux CGroup; 3. update some sentences or rephrase to make description clearer. Diffs - docs/learn/documentation/versioned/api/overview.md 6712344e84e19883b857e00549db2acb101c7e0e docs/learn/documentation/versioned/comparisons/mupd8.md 53417f91019c08d470d101685b6c40dad112bb18 docs/learn/documentation/versioned/comparisons/storm.md 58cb50895e7a419a7bede8911cd5b7f21fb487c4 docs/learn/documentation/versioned/container/coordinator-stream.md 98c1a845cda5ca2a53ee58b55508fc93492b982d docs/learn/documentation/versioned/container/event-loop.md 116238312df7071747cbbc14bc9c46f558755195 docs/learn/documentation/versioned/container/metrics.md 0423155537b0095b9efcfb40757e8de17b640ffd docs/learn/documentation/versioned/container/samza-container.md 55bec9813ca006757db046e6e85259b4d2e8c770 docs/learn/documentation/versioned/container/state-management.md 320392cd4412b459a093ab9657b7264cee42958b docs/learn/documentation/versioned/container/windowing.md b10e5d4eba61cb5da026af828d89d558457a6b16 docs/learn/documentation/versioned/hdfs/producer.md 0865a351b48c0b00e32282d2d7e44dfeae09b4ec docs/learn/documentation/versioned/index.html 1e79bd6f39ee67b5a2b74222d647342990bbbe23 docs/learn/documentation/versioned/yarn/isolation.md a76f6fd0f93482c085b15bb1b2c313a913508bcc docs/learn/documentation/versioned/yarn/yarn-host-affinity.md af6367da4f5611e208c9fb8426554c4a9c4ad6c8 Diff: https://reviews.apache.org/r/50317/diff/ Testing --- Basic unit tests and checkstyleTest passed. 1. Run unit tests: ./gradlew clean build 2. Check if your code follows the coding conventions: ./gradlew checkstyleMain checkstyleTest Brought up Jekyll, built a local website out of markdown pages, and verified the changes and links on localhost server. Thanks, Fred Ji
Re: Review Request 50317: SAMZA-978: update md files to resolve inconsistent links, broken links and some confusing sentences
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50317/ --- (Updated July 22, 2016, 11:10 p.m.) Review request for samza, Jon Bringhurst, Jagadish Venkatraman, and Yi Pan (Data Infrastructure). Changes --- linked up with https://issues.apache.org/jira/browse/SAMZA-978 Bugs: https://issues.apache.org/jira/browse/SAMZA-978 https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/SAMZA-978 Repository: samza Description --- 1. resolve the inconsistent next section link at the end of each section on samza.apache.org/learn/documentation/0.10/; 2. resolove some broken links such as Coda Hale's metrics library and Linux CGroup; 3. update some sentences or rephrase to make description clearer. Diffs - docs/learn/documentation/versioned/api/overview.md 6712344e84e19883b857e00549db2acb101c7e0e docs/learn/documentation/versioned/comparisons/mupd8.md 53417f91019c08d470d101685b6c40dad112bb18 docs/learn/documentation/versioned/comparisons/storm.md 58cb50895e7a419a7bede8911cd5b7f21fb487c4 docs/learn/documentation/versioned/container/coordinator-stream.md 98c1a845cda5ca2a53ee58b55508fc93492b982d docs/learn/documentation/versioned/container/event-loop.md 116238312df7071747cbbc14bc9c46f558755195 docs/learn/documentation/versioned/container/metrics.md 0423155537b0095b9efcfb40757e8de17b640ffd docs/learn/documentation/versioned/container/samza-container.md 55bec9813ca006757db046e6e85259b4d2e8c770 docs/learn/documentation/versioned/container/state-management.md 320392cd4412b459a093ab9657b7264cee42958b docs/learn/documentation/versioned/container/windowing.md b10e5d4eba61cb5da026af828d89d558457a6b16 docs/learn/documentation/versioned/hdfs/producer.md 0865a351b48c0b00e32282d2d7e44dfeae09b4ec docs/learn/documentation/versioned/index.html 1e79bd6f39ee67b5a2b74222d647342990bbbe23 docs/learn/documentation/versioned/yarn/isolation.md a76f6fd0f93482c085b15bb1b2c313a913508bcc docs/learn/documentation/versioned/yarn/yarn-host-affinity.md af6367da4f5611e208c9fb8426554c4a9c4ad6c8 Diff: https://reviews.apache.org/r/50317/diff/ Testing --- Basic unit tests and checkstyleTest passed. 1. Run unit tests: ./gradlew clean build 2. Check if your code follows the coding conventions: ./gradlew checkstyleMain checkstyleTest Brought up Jekyll, built a local website out of markdown pages, and verified the changes and links on localhost server. Thanks, Fred Ji
Re: Review Request 50317: SAMZA-978: update md files to resolve inconsistent links, broken links and some confusing sentences
> On July 22, 2016, 3:39 a.m., Jagadish Venkatraman wrote: > > docs/learn/documentation/versioned/container/samza-container.md, line 107 > > <https://reviews.apache.org/r/50317/diff/1/?file=1450702#file1450702line107> > > > > For consistency, we can maybe use the name broadcast-stream1 and > > broadcast-stream2. What do you think? Thanks for the comments. I updated them with broadcast-stream-1 and broadcast-stream-2. I also tested the changes and links on local Jekyll, with Navina's help. Thanks! - Fred --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50317/#review143173 --- On July 22, 2016, 11:06 p.m., Fred Ji wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50317/ > --- > > (Updated July 22, 2016, 11:06 p.m.) > > > Review request for samza, Jon Bringhurst, Jagadish Venkatraman, and Yi Pan > (Data Infrastructure). > > > Repository: samza > > > Description > --- > > 1. resolve the inconsistent next section link at the end of each section on > samza.apache.org/learn/documentation/0.10/; > 2. resolove some broken links such as Coda Hale's metrics library and Linux > CGroup; > 3. update some sentences or rephrase to make description clearer. > > > Diffs > - > > docs/learn/documentation/versioned/api/overview.md > 6712344e84e19883b857e00549db2acb101c7e0e > docs/learn/documentation/versioned/comparisons/mupd8.md > 53417f91019c08d470d101685b6c40dad112bb18 > docs/learn/documentation/versioned/comparisons/storm.md > 58cb50895e7a419a7bede8911cd5b7f21fb487c4 > docs/learn/documentation/versioned/container/coordinator-stream.md > 98c1a845cda5ca2a53ee58b55508fc93492b982d > docs/learn/documentation/versioned/container/event-loop.md > 116238312df7071747cbbc14bc9c46f558755195 > docs/learn/documentation/versioned/container/metrics.md > 0423155537b0095b9efcfb40757e8de17b640ffd > docs/learn/documentation/versioned/container/samza-container.md > 55bec9813ca006757db046e6e85259b4d2e8c770 > docs/learn/documentation/versioned/container/state-management.md > 320392cd4412b459a093ab9657b7264cee42958b > docs/learn/documentation/versioned/container/windowing.md > b10e5d4eba61cb5da026af828d89d558457a6b16 > docs/learn/documentation/versioned/hdfs/producer.md > 0865a351b48c0b00e32282d2d7e44dfeae09b4ec > docs/learn/documentation/versioned/index.html > 1e79bd6f39ee67b5a2b74222d647342990bbbe23 > docs/learn/documentation/versioned/yarn/isolation.md > a76f6fd0f93482c085b15bb1b2c313a913508bcc > docs/learn/documentation/versioned/yarn/yarn-host-affinity.md > af6367da4f5611e208c9fb8426554c4a9c4ad6c8 > > Diff: https://reviews.apache.org/r/50317/diff/ > > > Testing > --- > > Basic unit tests and checkstyleTest passed. > 1. Run unit tests: > ./gradlew clean build > 2. Check if your code follows the coding conventions: > ./gradlew checkstyleMain checkstyleTest > > Brought up Jekyll, built a local website out of markdown pages, and verified > the changes and links on localhost server. > > > Thanks, > > Fred Ji > >
Re: Review Request 50317: SAMZA-978: update md files to resolve inconsistent links, broken links and some confusing sentences
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50317/ --- (Updated July 22, 2016, 11:06 p.m.) Review request for samza, Jon Bringhurst, Jagadish Venkatraman, and Yi Pan (Data Infrastructure). Changes --- changed global-stream to broadstream to be consistent in samza-container.md. Tested with Jekyll. Repository: samza Description --- 1. resolve the inconsistent next section link at the end of each section on samza.apache.org/learn/documentation/0.10/; 2. resolove some broken links such as Coda Hale's metrics library and Linux CGroup; 3. update some sentences or rephrase to make description clearer. Diffs (updated) - docs/learn/documentation/versioned/api/overview.md 6712344e84e19883b857e00549db2acb101c7e0e docs/learn/documentation/versioned/comparisons/mupd8.md 53417f91019c08d470d101685b6c40dad112bb18 docs/learn/documentation/versioned/comparisons/storm.md 58cb50895e7a419a7bede8911cd5b7f21fb487c4 docs/learn/documentation/versioned/container/coordinator-stream.md 98c1a845cda5ca2a53ee58b55508fc93492b982d docs/learn/documentation/versioned/container/event-loop.md 116238312df7071747cbbc14bc9c46f558755195 docs/learn/documentation/versioned/container/metrics.md 0423155537b0095b9efcfb40757e8de17b640ffd docs/learn/documentation/versioned/container/samza-container.md 55bec9813ca006757db046e6e85259b4d2e8c770 docs/learn/documentation/versioned/container/state-management.md 320392cd4412b459a093ab9657b7264cee42958b docs/learn/documentation/versioned/container/windowing.md b10e5d4eba61cb5da026af828d89d558457a6b16 docs/learn/documentation/versioned/hdfs/producer.md 0865a351b48c0b00e32282d2d7e44dfeae09b4ec docs/learn/documentation/versioned/index.html 1e79bd6f39ee67b5a2b74222d647342990bbbe23 docs/learn/documentation/versioned/yarn/isolation.md a76f6fd0f93482c085b15bb1b2c313a913508bcc docs/learn/documentation/versioned/yarn/yarn-host-affinity.md af6367da4f5611e208c9fb8426554c4a9c4ad6c8 Diff: https://reviews.apache.org/r/50317/diff/ Testing (updated) --- Basic unit tests and checkstyleTest passed. 1. Run unit tests: ./gradlew clean build 2. Check if your code follows the coding conventions: ./gradlew checkstyleMain checkstyleTest Brought up Jekyll, built a local website out of markdown pages, and verified the changes and links on localhost server. Thanks, Fred Ji
Re: Review Request 50317: SAMZA-978: update md files to resolve inconsistent links, broken links and some confusing sentences
> On July 22, 2016, 12:37 a.m., Jake Maes wrote: > > docs/learn/documentation/versioned/index.html, lines 48-49 > > <https://reviews.apache.org/r/50317/diff/1/?file=1450706#file1450706line48> > > > > I'm not sure these should be nested in the Overview page. They are > > critical parts of the API and we want users to be able to find them easily. Thanks for the comments. These two are on the main list when we go to https://samza.apache.org/ (left column list including "Documentation" which is for this doc, and "Configuration", "Javadocs", and etc.), so users can find them as easily as "Hello Samza", "FAQ". We may not want to list "Configuration" and "Javadocs" again in the table for https://samza.apache.org/learn/documentation/0.10/ as another individual sections, and also, all sections in https://samza.apache.org/learn/documentation/0.10/ are following the same format except "Configuration" and "Javadocs". - Fred --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50317/#review143165 --- On July 22, 2016, 12:26 a.m., Fred Ji wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50317/ > --- > > (Updated July 22, 2016, 12:26 a.m.) > > > Review request for samza, Jon Bringhurst, Jagadish Venkatraman, and Yi Pan > (Data Infrastructure). > > > Repository: samza > > > Description > --- > > 1. resolve the inconsistent next section link at the end of each section on > samza.apache.org/learn/documentation/0.10/; > 2. resolove some broken links such as Coda Hale's metrics library and Linux > CGroup; > 3. update some sentences or rephrase to make description clearer. > > > Diffs > - > > docs/learn/documentation/versioned/api/overview.md > 6712344e84e19883b857e00549db2acb101c7e0e > docs/learn/documentation/versioned/comparisons/mupd8.md > 53417f91019c08d470d101685b6c40dad112bb18 > docs/learn/documentation/versioned/comparisons/storm.md > 58cb50895e7a419a7bede8911cd5b7f21fb487c4 > docs/learn/documentation/versioned/container/coordinator-stream.md > 98c1a845cda5ca2a53ee58b55508fc93492b982d > docs/learn/documentation/versioned/container/event-loop.md > 116238312df7071747cbbc14bc9c46f558755195 > docs/learn/documentation/versioned/container/metrics.md > 0423155537b0095b9efcfb40757e8de17b640ffd > docs/learn/documentation/versioned/container/samza-container.md > 55bec9813ca006757db046e6e85259b4d2e8c770 > docs/learn/documentation/versioned/container/state-management.md > 320392cd4412b459a093ab9657b7264cee42958b > docs/learn/documentation/versioned/container/windowing.md > b10e5d4eba61cb5da026af828d89d558457a6b16 > docs/learn/documentation/versioned/hdfs/producer.md > 0865a351b48c0b00e32282d2d7e44dfeae09b4ec > docs/learn/documentation/versioned/index.html > 1e79bd6f39ee67b5a2b74222d647342990bbbe23 > docs/learn/documentation/versioned/yarn/isolation.md > a76f6fd0f93482c085b15bb1b2c313a913508bcc > docs/learn/documentation/versioned/yarn/yarn-host-affinity.md > af6367da4f5611e208c9fb8426554c4a9c4ad6c8 > > Diff: https://reviews.apache.org/r/50317/diff/ > > > Testing > --- > > Basic unit tests and checkstyleTest passed. > 1. Run unit tests: > ./gradlew clean build > 2. Check if your code follows the coding conventions: > ./gradlew checkstyleMain checkstyleTest > > > Thanks, > > Fred Ji > >
Re: Review Request 50317: SAMZA-978: update md files to resolve inconsistent links, broken links and some confusing sentences
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50317/ --- (Updated July 22, 2016, 12:26 a.m.) Review request for samza, Jon Bringhurst, Jagadish Venkatraman, and Yi Pan (Data Infrastructure). Changes --- added tests done and added more people in the reviewers list. Repository: samza Description (updated) --- 1. resolve the inconsistent next section link at the end of each section on samza.apache.org/learn/documentation/0.10/; 2. resolove some broken links such as Coda Hale's metrics library and Linux CGroup; 3. update some sentences or rephrase to make description clearer. Diffs - docs/learn/documentation/versioned/api/overview.md 6712344e84e19883b857e00549db2acb101c7e0e docs/learn/documentation/versioned/comparisons/mupd8.md 53417f91019c08d470d101685b6c40dad112bb18 docs/learn/documentation/versioned/comparisons/storm.md 58cb50895e7a419a7bede8911cd5b7f21fb487c4 docs/learn/documentation/versioned/container/coordinator-stream.md 98c1a845cda5ca2a53ee58b55508fc93492b982d docs/learn/documentation/versioned/container/event-loop.md 116238312df7071747cbbc14bc9c46f558755195 docs/learn/documentation/versioned/container/metrics.md 0423155537b0095b9efcfb40757e8de17b640ffd docs/learn/documentation/versioned/container/samza-container.md 55bec9813ca006757db046e6e85259b4d2e8c770 docs/learn/documentation/versioned/container/state-management.md 320392cd4412b459a093ab9657b7264cee42958b docs/learn/documentation/versioned/container/windowing.md b10e5d4eba61cb5da026af828d89d558457a6b16 docs/learn/documentation/versioned/hdfs/producer.md 0865a351b48c0b00e32282d2d7e44dfeae09b4ec docs/learn/documentation/versioned/index.html 1e79bd6f39ee67b5a2b74222d647342990bbbe23 docs/learn/documentation/versioned/yarn/isolation.md a76f6fd0f93482c085b15bb1b2c313a913508bcc docs/learn/documentation/versioned/yarn/yarn-host-affinity.md af6367da4f5611e208c9fb8426554c4a9c4ad6c8 Diff: https://reviews.apache.org/r/50317/diff/ Testing (updated) --- Basic unit tests and checkstyleTest passed. 1. Run unit tests: ./gradlew clean build 2. Check if your code follows the coding conventions: ./gradlew checkstyleMain checkstyleTest Thanks, Fred Ji
Review Request 50317: SAMZA-978: update md files to resolve inconsistent links, broken links and some confusing sentences
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50317/ --- Review request for samza. Repository: samza Description --- 1. resolve the inconsistent next section link at the end of each section on samza.apache.org/learn/documentation/0.10/; 2. resolove some broken links such as Coda Hale's metrics library and Linux CGroup; 3. update some sentences or rephrase to make description clearer. Diffs - docs/learn/documentation/versioned/api/overview.md 6712344e84e19883b857e00549db2acb101c7e0e docs/learn/documentation/versioned/comparisons/mupd8.md 53417f91019c08d470d101685b6c40dad112bb18 docs/learn/documentation/versioned/comparisons/storm.md 58cb50895e7a419a7bede8911cd5b7f21fb487c4 docs/learn/documentation/versioned/container/coordinator-stream.md 98c1a845cda5ca2a53ee58b55508fc93492b982d docs/learn/documentation/versioned/container/event-loop.md 116238312df7071747cbbc14bc9c46f558755195 docs/learn/documentation/versioned/container/metrics.md 0423155537b0095b9efcfb40757e8de17b640ffd docs/learn/documentation/versioned/container/samza-container.md 55bec9813ca006757db046e6e85259b4d2e8c770 docs/learn/documentation/versioned/container/state-management.md 320392cd4412b459a093ab9657b7264cee42958b docs/learn/documentation/versioned/container/windowing.md b10e5d4eba61cb5da026af828d89d558457a6b16 docs/learn/documentation/versioned/hdfs/producer.md 0865a351b48c0b00e32282d2d7e44dfeae09b4ec docs/learn/documentation/versioned/index.html 1e79bd6f39ee67b5a2b74222d647342990bbbe23 docs/learn/documentation/versioned/yarn/isolation.md a76f6fd0f93482c085b15bb1b2c313a913508bcc docs/learn/documentation/versioned/yarn/yarn-host-affinity.md af6367da4f5611e208c9fb8426554c4a9c4ad6c8 Diff: https://reviews.apache.org/r/50317/diff/ Testing --- Thanks, Fred Ji
Re: "publishToMavenLocal" task not found in gradlew
Hi Ankita, The command "./gradlew publishToMavenLocal" is to be run in Samza project, not "hello-samza" project. In your case, you may just skip this command since "bin/grid bootstrap" would already build a Samza package for you (hello-samza/deploy/samza). Thanks, Fred On Thu, Jul 21, 2016 at 2:15 PM, Ankita Karandikarwrote: > Hi, > > While running the project Hello-Samza from the tutorials provided on the > web, I was unable to fire the following command on my terminal: > > * > ./gradlew publishToMavenLocal > *** > > Please suggest. > > Note: Please find below the result of the command ./gradlew tasks for your > reference > > > > *** > > Build tasks > > --- > > assemble - Assembles the outputs of this project. > > build - Assembles and tests this project. > > buildDependents - Assembles and tests this project and all projects that > depend on it. > > buildNeeded - Assembles and tests this project and all projects it depends > on. > > classes - Assembles classes 'main'. > > clean - Deletes the build directory. > > jar - Assembles a jar archive containing the main classes. > > testClasses - Assembles classes 'test'. > > > Build Setup tasks > > - > > init - Initializes a new Gradle build. [incubating] > > > Documentation tasks > > --- > > javadoc - Generates Javadoc API documentation for the main source code. > > > Help tasks > > -- > > components - Displays the components produced by root project > 'hello-samza'. [incubating] > > dependencies - Displays all dependencies declared in root project > 'hello-samza'. > > dependencyInsight - Displays the insight into a specific dependency in root > project 'hello-samza'. > > help - Displays a help message. > > projects - Displays the sub-projects of root project 'hello-samza'. > > properties - Displays the properties of root project 'hello-samza'. > > tasks - Displays the tasks runnable from root project 'hello-samza'. > > > IDE tasks > > - > > cleanEclipse - Cleans all Eclipse files. > > eclipse - Generates all Eclipse files. > > > Verification tasks > > -- > > check - Runs all checks. > > test - Runs the unit tests. > > > Other tasks > > --- > > dumpWikiEdits > > dumpWikiRaw > > dumpWikiStats > > listKafkaTopics > > runWikiFeed > > runWikiParser > > runWikiStats > > stopGrid > > wrapper - Updates gradlew and supporting files. > > > > > AKARANDIKAR-E366-MPR13:hello-samza akarandikar$ ./gradlew tasks | grep > publishToMavenLocal > > AKARANDIKAR-E366-MPR13:hello-samza akarandikar$ > > *** > > Thanks. > > Regards, > > Ankita >
Re: Review Request 49877: SAMZA-972: Holistic memory monitoring for SamzaContainer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49877/#review141989 --- Thanks for the RB. I have a few comments for clarification. samza-core/src/main/java/org/apache/samza/container/host/ProcfsBasedStatisticsMonitor.java (line 63) <https://reviews.apache.org/r/49877/#comment207455> [Info Question] If only supported in Linux, do you want to do a linux check by using System.getProperty("os.name")? samza-core/src/main/java/org/apache/samza/container/host/ProcfsBasedStatisticsMonitor.java (line 72) <https://reviews.apache.org/r/49877/#comment207452> For the numbers matched, are we trying to match just valid numbers including negative numbers? If so, would it be better to use "-?[0-9]+" to be more specific? Also, if we are matching the whole, it is better to have "$" in the end since we are using "^" in the beginning. samza-core/src/main/java/org/apache/samza/container/host/ProcfsBasedStatisticsMonitor.java (line 141) <https://reviews.apache.org/r/49877/#comment207453> Is it possible that the file was not created due to other reasons, such as permission and etc.? If so, then this message might not be accurate. samza-core/src/main/java/org/apache/samza/container/host/ProcfsBasedStatisticsMonitor.java (line 206) <https://reviews.apache.org/r/49877/#comment207454> Do we want to log if match is not found? This might help us to capture the issue if the format is changed for the path somehow in the future. - Fred Ji On July 12, 2016, 12:17 a.m., Jagadish Venkatraman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49877/ > --- > > (Updated July 12, 2016, 12:17 a.m.) > > > Review request for samza, Boris Shkolnik, Chris Pettitt, Fred Ji, Jake Maes, > Yi Pan (Data Infrastructure), and Navina Ramesh. > > > Repository: samza > > > Description > --- > > This feature introduces physical memory monitoring in SamzaContainer. > > Context: > Often memory used by the SamzaContainer process includes > A. JVM Heap memory: This is where all JVM variables live. > B. Native memory: This memory lives out of the JVM heap and is not visible to > the JVM. Examples include used by RocksDb, native libraries that user code > depends on etc. > > User jobs could be killed by Yarn if their total memory (A+B) exceeds the > configured maximum of yarn.container.memory.mb. > > Currently, while our existing metrics provide visibility into [A] via JMX, we > don't have visibility into [B]. (as it's totally external to the JVM). > > This feature uses Linux ProcFS to provide a complete view of the memory (both > A & B) to help Samza users understand memory better. (Schedulers like Apache > Yarn that require a holistic view of memory (A+B) also use ProcFS. For the > curious, here's the Yarn implementation - > http://grepcode.com/file/repo1.maven.org/maven2/org.apache.hadoop/hadoop-yarn-common/0.23.1/org/apache/hadoop/yarn/util/ProcfsBasedProcessTree.java > that inspired this idea) > > Scope: The scope of this RB is only to Linux distributions. (Mac based > implementation may be a separate change list.) > > > Diffs > - > > build.gradle ba4a9d14fe24e1ff170873920cd5eeef656955af > checkstyle/import-control.xml 325c38131047836dc8aedaea4187598ef3ba7666 > > samza-core/src/main/java/org/apache/samza/container/host/ProcfsBasedStatisticsMonitor.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/container/host/SystemStatistics.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/container/host/SystemStatisticsMonitor.java > PRE-CREATION > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > 18c09224bbae959342daf9b2b7a7d971cc224f48 > > samza-core/src/main/scala/org/apache/samza/container/SamzaContainerMetrics.scala > 2044ce01ffded8434e762d99355d5df43642c66b > > samza-core/src/test/java/org/apache/samza/container/host/TestProcfsBasedStatisticsMonitor.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/49877/diff/ > > > Testing > --- > > 1. Unit tests with mock PROC-FS snapshots of processes > 2. Deployed actual jobs on my dev box. >2.1 Obtained the operating system's view of the container memory using > 'ps' and other tools. >2.2 Verified that the total memory reported by the monitor is the same as > the OS's view of memory[2.1] > 3. Tested on various Linux distributions I could find internally: > - RHEL release 6.4, 6.5, 6.6 (Santiago) > > > Thanks, > > Jagadish Venkatraman > >
Re: Review Request 48356: RFC: Samza as a library
> On July 12, 2016, 12:15 a.m., Fred Ji wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java, > > line 92 > > <https://reviews.apache.org/r/48356/diff/7/?file=1441779#file1441779line92> > > > > Maybe if(!StringUtils.isBlank())? Since we would like to have a valid > > name. > > Navina Ramesh wrote: > Is StringUtils part of java.lang? Or are you referring to some other > library? > > Fred Ji wrote: > It is org.apache.commons.lang.StringUtils > > Navina Ramesh wrote: > Fred, We already have guava commons added. I don't want to overload the > core package. For now, it is understood that a config value cannot be empty > or whitespace. Let's leave the checks as it is for now. I see. If we use guava commons lib, there is a better similar static function: Strings.isNullOrEmpty(@Nullable String string) - Fred --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/#review141799 --- On July 12, 2016, 10:02 p.m., Navina Ramesh wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48356/ > --- > > (Updated July 12, 2016, 10:02 p.m.) > > > Review request for samza, Chris Pettitt and Yi Pan (Data Infrastructure). > > > Repository: samza > > > Description > --- > > Added ConfigBuilder and support classes > > Added JobCoordinator interfaces > > > Adding StreamProcessor, StandaloneJobCoordinator and updating SamzaContainer > interface > > > Added TestStreamProcessor and some unit tests for ConfigBuilders > > > Changing who defined processorId > > > Fixed checkstyle errors > > > Replaced SamzaException with ConfigException > > > Removing localityManager instantiation from Samza Container > > > Diffs > - > > build.gradle ba4a9d14fe24e1ff170873920cd5eeef656955af > checkstyle/import-control.xml 325c38131047836dc8aedaea4187598ef3ba7666 > samza-core/src/main/java/org/apache/samza/config/TaskConfigJava.java > 021d42a70179f5d14f51ac87cb09dcc97218095e > > samza-core/src/main/java/org/apache/samza/configbuilder/CheckpointConfig.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/configbuilder/GenericConfigBuilder.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/configbuilder/KafkaCheckpointConfig.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/configbuilder/KafkaSystemConfig.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/configbuilder/SerdeConfig.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/configbuilder/StandaloneConfigBuilder.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/configbuilder/SystemConfig.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/container/grouper/stream/AllSspToSingleTaskGrouper.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/container/grouper/stream/AllSspToSingleTaskGrouperFactory.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/container/grouper/task/SingleContainerGrouper.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/container/grouper/task/SingleContainerGrouperFactory.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/coordinator/JobCoordinator.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/coordinator/JobCoordinatorFactory.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/processor/StreamProcessor.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/standalone/StandaloneJobCoordinator.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/standalone/StandaloneJobCoordinatorFactory.java > PRE-CREATION > samza-core/src/main/scala/org/apache/samza/config/JobConfig.scala > 49b08f6b68dbb44757dcc8ce8d60c365a9d22981 > samza-core/src/main/scala/org/apache/samza/config/TaskConfig.scala > 08a4debb06f9925ae741049abb2ee0df97b2243b > samza-core/src/main/scala/org/apache/samza/container/RunLoop.scala > cf05c15c836ddfa54ba8fe27abc18ed88ac5fc11 > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > 18c09224bbae959342daf9b2b7a7d971cc224f48 > samza-core/src/main/s
Re: Review Request 48356: RFC: Samza as a library
> On July 12, 2016, 12:15 a.m., Fred Ji wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java, > > line 92 > > <https://reviews.apache.org/r/48356/diff/7/?file=1441779#file1441779line92> > > > > Maybe if(!StringUtils.isBlank())? Since we would like to have a valid > > name. > > Navina Ramesh wrote: > Is StringUtils part of java.lang? Or are you referring to some other > library? It is org.apache.commons.lang.StringUtils > On July 12, 2016, 12:15 a.m., Fred Ji wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java, > > line 154 > > <https://reviews.apache.org/r/48356/diff/7/?file=1441779#file1441779line154> > > > > Or just use StringUtils.isEmpty(streamName) or even > > StringUtils.isBlank(streamName)? > > Navina Ramesh wrote: > Is StringUtils part of java.lang? Or are you referring to some other > library? It is org.apache.commons.lang.StringUtils > On July 12, 2016, 12:15 a.m., Fred Ji wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/SerdeConfig.java, > > line 43 > > <https://reviews.apache.org/r/48356/diff/7/?file=1441783#file1441783line43> > > > > Could serdeAlias be null accidentally? If so, it would be better to > > have a null check for serdeAlias. > > Navina Ramesh wrote: > SerdeAlias is an enum. I don't think you can specify a "null" value for > enum. null can be assigned to an enum in Java. > On July 12, 2016, 12:15 a.m., Fred Ji wrote: > > samza-core/src/main/java/org/apache/samza/config/TaskConfigJava.java, line > > 131 > > <https://reviews.apache.org/r/48356/diff/7/?file=1441777#file1441777line131> > > > > [INFO Question] Just curious, Is there a required code convention for > > samza code base here? Do we do the following pattern? > > > > if () { > > ; > > } > > Navina Ramesh wrote: > I don't believe we have been strict on coding convention. The convention > you suggested is what, I believe, is used at LinkedIn. We are not very stric > on open-source. As long as the code is readable, it is ok. Sounds good to me. Thanks! - Fred --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/#review141799 --- On July 12, 2016, 6:45 p.m., Navina Ramesh wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48356/ > --- > > (Updated July 12, 2016, 6:45 p.m.) > > > Review request for samza, Chris Pettitt and Yi Pan (Data Infrastructure). > > > Repository: samza > > > Description > --- > > Added ConfigBuilder and support classes > > Added JobCoordinator interfaces > > > Adding StreamProcessor, StandaloneJobCoordinator and updating SamzaContainer > interface > > > Added TestStreamProcessor and some unit tests for ConfigBuilders > > > Changing who defined processorId > > > Fixed checkstyle errors > > > Replaced SamzaException with ConfigException > > > Removing localityManager instantiation from Samza Container > > > Diffs > - > > build.gradle ba4a9d14fe24e1ff170873920cd5eeef656955af > checkstyle/import-control.xml 325c38131047836dc8aedaea4187598ef3ba7666 > samza-core/src/main/java/org/apache/samza/config/TaskConfigJava.java > 021d42a70179f5d14f51ac87cb09dcc97218095e > > samza-core/src/main/java/org/apache/samza/configbuilder/CheckpointConfig.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/configbuilder/GenericConfigBuilder.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/configbuilder/KafkaCheckpointConfig.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/configbuilder/KafkaSystemConfig.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/configbuilder/SerdeConfig.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/configbuilder/StandaloneConfigBuilder.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/configbuilder/SystemConfig.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/container/grouper/stream/AllSspToSingleTaskGrouper.java > PRE-CREATION > > sam
Re: Review Request 48356: RFC: Samza as a library
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/#review141799 --- Thanks a lot for the RB! I only have a few INFO questions to learn more details and MINOR comments as below. Thanks! samza-core/src/main/java/org/apache/samza/config/TaskConfigJava.java (line 131) <https://reviews.apache.org/r/48356/#comment207185> [INFO Question] Just curious, Is there a required code convention for samza code base here? Do we do the following pattern? if () { ; } samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java (line 92) <https://reviews.apache.org/r/48356/#comment207186> Maybe if(!StringUtils.isBlank())? Since we would like to have a valid name. samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java (line 144) <https://reviews.apache.org/r/48356/#comment207188> Would it be more accurate to call it STREAM_KEY_SERDE_FORMATTING_STRING instead of STREAM_KEY_SERDE_REGEX? regex has a different rule. samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java (line 154) <https://reviews.apache.org/r/48356/#comment207187> Or just use StringUtils.isEmpty(streamName) or even StringUtils.isBlank(streamName)? samza-core/src/main/java/org/apache/samza/configbuilder/KafkaCheckpointConfig.java (line 28) <https://reviews.apache.org/r/48356/#comment207190> [Info Question] what is the reason to have 3 and 26214400 as default values here? samza-core/src/main/java/org/apache/samza/configbuilder/KafkaCheckpointConfig.java (line 59) <https://reviews.apache.org/r/48356/#comment207191> Why do we have systemName here since we have systemConfig in the result map which has systemName already? samza-core/src/main/java/org/apache/samza/configbuilder/KafkaSystemConfig.java (line 26) <https://reviews.apache.org/r/48356/#comment207192> Similar to a previous comment. Maybe more proper to call it FORMATTING_STRING instead of REGEX? samza-core/src/main/java/org/apache/samza/configbuilder/SerdeConfig.java (line 43) <https://reviews.apache.org/r/48356/#comment207193> Could serdeAlias be null accidentally? If so, it would be better to have a null check for serdeAlias. samza-core/src/main/java/org/apache/samza/container/grouper/stream/AllSspToSingleTaskGrouper.java (line 34) <https://reviews.apache.org/r/48356/#comment207194> Could ssps be null? samza-core/src/main/java/org/apache/samza/processor/StreamProcessor.java (line 166) <https://reviews.apache.org/r/48356/#comment207195> Would it be better to log separately for these two exceptions for trouble shooting purpose if issues happen? samza-core/src/main/scala/org/apache/samza/config/TaskConfig.scala (line 29) <https://reviews.apache.org/r/48356/#comment207196> I know it is from previous code, but do you mind explaining or putting a comment regarding what -1L means here? - Fred Ji On July 12, 2016, midnight, Navina Ramesh wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48356/ > --- > > (Updated July 12, 2016, midnight) > > > Review request for samza, Chris Pettitt and Yi Pan (Data Infrastructure). > > > Repository: samza > > > Description > --- > > Added ConfigBuilder and support classes > > Added JobCoordinator interfaces > > > Adding StreamProcessor, StandaloneJobCoordinator and updating SamzaContainer > interface > > > Added TestStreamProcessor and some unit tests for ConfigBuilders > > > Changing who defined processorId > > > Fixed checkstyle errors > > > Replaced SamzaException with ConfigException > > > Removing localityManager instantiation from Samza Container > > > Diffs > - > > build.gradle ba4a9d14fe24e1ff170873920cd5eeef656955af > checkstyle/import-control.xml 325c38131047836dc8aedaea4187598ef3ba7666 > samza-core/src/main/java/org/apache/samza/config/TaskConfigJava.java > 021d42a70179f5d14f51ac87cb09dcc97218095e > > samza-core/src/main/java/org/apache/samza/configbuilder/CheckpointConfig.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/configbuilder/GenericConfigBuilder.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/configbuilder/KafkaCheckpointConfig.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/configbuilder/