RE: [DISCUSS] SAMZA-1141 - Apache Samza Development Process Improvements

[Garry Turkington]

Very much in favour of the proposed SEP process -- strong +1 from me.

-Original Message-
From: Navina Ramesh (Apache) [mailto:nav...@apache.org] 
Sent: Tuesday, March 14, 2017 10:16 PM
To: dev@samza.apache.org
Subject: [DISCUSS] SAMZA-1141 - Apache Samza Development Process Improvements

Hi everyone,

We switched to using Pull Requests for code reviews a few months back.
Clearly, there are some drawbacks to that model and we are trying to address 
the shortcomings. I have gathered input from some of the committers regarding 
what is missing the code review process and what can be improved.
Please take a look and provide feedback.

Additionally, we are considering moving to a KIP/FLIP-like model for submitting 
design proposals (major changes to samza). Lately, there have been some major 
feature discussions that are not documented consistently in a centralized 
location. The proposal in SAMZA-1141 
 address the design review 
process as well. Please review it too. I have already created a wiki page 

describing the Samza Enhancement Proposal (SEP) process and an SEP template. 
Going forward, let's start adding all major change proposals to the wiki and 
discuss the design on the mailing list.

Your cooperation is highly appreciated during this period of transition in the 
process :)

Feedbacks welcome!

Thanks!
--
Navina R

PS: Alternatives name suggestions for "SEP" are welcome !

-
No virus found in this message.
Checked by AVG - www.avg.com
Version: 2016.0.7998 / Virus Database: 4756/14112 - Release Date: 03/13/17

[GitHub] samza pull request #75: SAMZA-1067: Physical execution graph and planner for...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/samza/pull/75


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Review Request 52168: Tasks endpoint to list the complete details of all tasks related to a job

[Xinyu Liu]

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



Please rebase this patch with master.


samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala
Line 64 (original), 78 (patched)


Add java doc indicating we not only read jobmodel but update some other 
stuff.



samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala
Line 120 (original), 155 (patched)


make this private and don't expose it outside.



samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala
Line 188 (original), 224 (patched)


rename this as getJobModle. In javadoc, make clear there is no side effect.


- Xinyu Liu


On Feb. 16, 2017, 6:26 a.m., Shanthoosh Venkataraman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52168/
> ---
> 
> (Updated Feb. 16, 2017, 6:26 a.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> ---
> 
> This patch contains the following changes
>  * Http get api to list the complete details of all the tasks that belongs to 
> a job. 
>  * Refactored some methods in coordinator stream, to reuse the existing 
> functionality of getting jobConfig from the coordinator stream.
> 
> 
> Diffs
> -
> 
>   docs/learn/documentation/versioned/rest/resource-directory.md 
> 79746d1e2eb3491e4bd26c3c7cf6c7efd150d8ef 
>   docs/learn/documentation/versioned/rest/resources/tasks.md PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala 
> df63b97e9d598ecd1840111ba490a723e410d089 
>   samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala 
> 022b480856483059cb9f837a08f97a718bc04c31 
>   samza-core/src/main/scala/org/apache/samza/util/Util.scala 
> c4836f202f7eda1d4e71eac94fd48e46207b0316 
>   samza-rest/src/main/config/samza-rest.properties 
> 7be0b47d1466d2199ae278247e8d81522fb6a91c 
>   samza-rest/src/main/java/org/apache/samza/rest/model/Partition.java 
> PRE-CREATION 
>   samza-rest/src/main/java/org/apache/samza/rest/model/Task.java PRE-CREATION 
>   
> samza-rest/src/main/java/org/apache/samza/rest/proxy/job/AbstractJobProxy.java
>  4d8647f3e1e650632e38b47ba5a8a2dac004f545 
>   
> samza-rest/src/main/java/org/apache/samza/rest/proxy/job/JobProxyFactory.java 
> 067711a74e5b0d7277a9c8b2d2517b56e9cfbcca 
>   
> samza-rest/src/main/java/org/apache/samza/rest/proxy/job/SimpleYarnJobProxy.java
>  a935c98730f85f448c688a6baf2e8ddffdbb2cb4 
>   
> samza-rest/src/main/java/org/apache/samza/rest/proxy/job/SimpleYarnJobProxyFactory.java
>  11d93d4608d23a4e3fb3bfc50dfac35ab6dbdf3c 
>   
> samza-rest/src/main/java/org/apache/samza/rest/proxy/task/SamzaTaskProxy.java 
> PRE-CREATION 
>   
> samza-rest/src/main/java/org/apache/samza/rest/proxy/task/SamzaTaskProxyFactory.java
>  PRE-CREATION 
>   samza-rest/src/main/java/org/apache/samza/rest/proxy/task/TaskProxy.java 
> PRE-CREATION 
>   
> samza-rest/src/main/java/org/apache/samza/rest/proxy/task/TaskProxyFactory.java
>  PRE-CREATION 
>   
> samza-rest/src/main/java/org/apache/samza/rest/proxy/task/TaskResourceConfig.java
>  PRE-CREATION 
>   
> samza-rest/src/main/java/org/apache/samza/rest/resources/BaseResourceConfig.java
>  PRE-CREATION 
>   
> samza-rest/src/main/java/org/apache/samza/rest/resources/DefaultResourceFactory.java
>  e0224c6bcf4aeaa336e5786ac472482507fcd382 
>   samza-rest/src/main/java/org/apache/samza/rest/resources/JobsResource.java 
> a566db598c284d69ea61af88fdc0851483d5a089 
>   
> samza-rest/src/main/java/org/apache/samza/rest/resources/JobsResourceConfig.java
>  527482d2ee55747e7b3f9c54c8a3b1afe7ad8797 
>   samza-rest/src/main/java/org/apache/samza/rest/resources/Responses.java 
> PRE-CREATION 
>   samza-rest/src/main/java/org/apache/samza/rest/resources/TasksResource.java 
> PRE-CREATION 
>   
> samza-rest/src/test/java/org/apache/samza/rest/resources/TestJobsResource.java
>  7db437b348ecd286185898b8f8ab0220d59da71a 
>   
> samza-rest/src/test/java/org/apache/samza/rest/resources/TestTasksResource.java
>  PRE-CREATION 
>   
> samza-rest/src/test/java/org/apache/samza/rest/resources/mock/MockInstallationFinder.java
>  PRE-CREATION 
>   
> samza-rest/src/test/java/org/apache/samza/rest/resources/mock/MockResourceFactory.java
>  PRE-CREATION 
>   
> samza-rest/src/test/java/org/apache/samza/rest/resources/mock/MockTaskProxy.java
>  PRE-CREATION 
>   
> samza-rest/src/test/java/org/apache/samza/rest/resources/mock/MockTaskProxyFactory.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/52168/diff/14/
> 

[GitHub] samza pull request #87: SAMZA-1146: TaskCallbackManager commit fix.

[shanthoosh]

GitHub user shanthoosh opened a pull request:

https://github.com/apache/samza/pull/87

SAMZA-1146: TaskCallbackManager commit fix.

Each task callback in samza belongs to different SystemStreamPartition. 
When multiple callbacks in contagious order are available for commit, callback 
with highest sequence number is chosen for commit. This will prevent 
checkpointing of completed callbacks that has commit request and doesn't have 
highest sequence number. Upon task restart this will lead to duplicate 
reprocessing of already processed messages (since completed callbacks for some 
SystemStreamPartition's aren't committed earlier).

This PR fixes it and commits all completed callbacks that has commit 
request defined. Added a test to verify the behavior.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/shanthoosh/samza Fixing_CallBackManager_Commit

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/samza/pull/87.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #87


commit 7afbbcef414bb2d3e64117a8e1880072e0a99471
Author: Shanthoosh Venkataraman 
Date:   2017-03-09T19:54:20Z

SAMZA-1146 : TaskCallbackManager commit fix.

Changing TaskCallbackManager.update to return all completed callbacks that 
has
commit request defined for commit(not just the callback with highest 
sequence number).




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] samza pull request #86: SAMZA:1145: Provide Ability To Confgure The Default ...

[jwlent55]

GitHub user jwlent55 opened a pull request:

https://github.com/apache/samza/pull/86

SAMZA:1145: Provide Ability To Confgure The Default Number Of Changel…

…og Replicas

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/TiVo/samza SAMZA-1145

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/samza/pull/86.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #86


commit 4acda381ed35358233f88d11c58402af671681da
Author: James Lent 
Date:   2017-03-15T21:58:41Z

SAMZA:1145: Provide Ability To Confgure The Default Number Of Changelog 
Replicas




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Understanding metrics

[Ankit Malhotra]

Hi Xinyu,

Super weird, but I can’t seem to find the commit-ns nor the window-ns metric 
for our jobs. There are window-calls and commit-calls but the timings are 
missing.

task.commit.ms=9
task.window.ms=9

Ankit
On 3/14/17, 8:00 PM, "xinyu liu"  wrote:

Hi, Ankit,

When running your job in multithreading, block-ns here actually includes
the process_ns. This is because after your task.process() is submitted to
the thread pool, the run loop thread will be blocked until the process() is
complete for one of the task. It's interesting that block-ns (0.3 ms) is
much longer than process-ns (0.12 ms). I am wondering whether you also have
window and checkpoint configured for your job. Since window and checkpoint
will also be running inside this thread pool to improve the parallelism,
block-ns will be affected since the run loop will block for
window/checkpoint to complete. If you are using window/commit, please send
us the configs (task.window.ms and task.commit.ms) and the timer metrics
(window-ns and commit-ns). Then we can correlate better with block-ns.

Thanks,
Xinyu

On Tue, Mar 14, 2017 at 3:33 PM, Ankit Malhotra 
wrote:

> Wait, block-ns = 0.3ms (300,000ns). Also, why are we not adding in
> choose-ns?
>
> Thanks
> Ankit
>
> On 3/14/17, 6:26 PM, "Jagadish Venkatraman" 
> wrote:
>
> I would expect (process_ns + block_ns) to be almost the same as 0.15
> which
> makes sense.
>
> process_ns = 0.12 ms
> block_ns = 0.03 ms
> process_ns + block_ns ~ 0.15ms
>
> This corresponds to the number of process calls roughly 1/7000 ~
> 0.15ms per
> process call.
>
> >> Each process call is a separate thread.
> Given that you have one partition in each container, and you want
> in-order
> processing, there will be only one thread that's processing messages.
> The
> two other threads are not doing work, and entail a constant (albeit
> insignificant) synchronization overhead.
>
>
>
>
>
> On Tue, Mar 14, 2017 at 3:03 PM, Ankit Malhotra <
> amalho...@appnexus.com>
> wrote:
>
> > Hi,
> >
> > We are trying to understand metrics that are being populated by our
> samza
> > job and are a little confused what each of these metrics mean
> especially
> > since we’re running the job with a thread pool.
> >
> >
> > · We have 3 input streams
> >
> > · job.container.thread.pool.size=3
> >
> > · 1 container per partition
> >
> > · Using a RocksDB backed store with changelogging
> >
> > · process-ns = 120,000
> >
> > · get-ns ~ 30,000
> >
> > · put-ns ~ 90,000
> >
> > · block-ns ~ 300,000
> >
> > · choose-ns ~ 500,000
> >
> > Metrics are avg(metric) for all containers/partitions.
> >
> > Process-envelopes ~ 7000/sec.
> >
> > If I back the math out, this correlates quite closely to process-ns.
> > (1/7000 ~ 0.15ms).
> >
> > What I don’t understand is that the event loop is single threaded.
> Even
> > though, each process call is a separate thread, the main even loop 
is
> > blocking (block-ns) and choosing (choose-ns) every time and these
> times are
> > quite high. Given these metrics, how is it that we are consistently
> seeing
> > the above metrics?
> >
> > Also, lag (messages behind high watermark) is ~ 0.
> >
> > Thanks
> > Ankit
> >
> >
> >
> >
> >
> >
>
>
> --
> Jagadish V,
> Graduate Student,
> Department of Computer Science,
> Stanford University
>
>
>