samza-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Guozhang Wang" <wangg...@gmail.com>
Subject Re: Review Request 35397: Fix SAMZA-697
Date Wed, 17 Jun 2015 23:14:50 GMT


> On June 17, 2015, 9:03 p.m., Yan Fang wrote:
> > samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java, line 75
> > <https://reviews.apache.org/r/35397/diff/2/?file=985834#file985834line75>
> >
> >     use "if" to be consistent?

I thought there may be some class paths that contain multiple dots, but after some checking
not I think it is not possible. So changing it.


> On June 17, 2015, 9:03 p.m., Yan Fang wrote:
> > samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java, lines 162-163
> > <https://reviews.apache.org/r/35397/diff/2/?file=985834#file985834line162>
> >
> >     doc may not be very precise. It's also possible that this class is blacklisted
not ClassNotFoundException, right? I guess this is the reason you do not put line 164 in line
156 to simplify the code.

If the class is blacklisted, we will just avoid loading it in the current classloader, and
we should expect it to be loadable in the parent classloader. So I think the comment here
is correct.


> On June 17, 2015, 9:03 p.m., Yan Fang wrote:
> > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala, lines
426-428
> > <https://reviews.apache.org/r/35397/diff/2/?file=985836#file985836line426>
> >
> >     do we want the TaskClassLoader to take care of the StreamTask itself? I thought
we only used the TaskClassLoader to take care of what happens *inside* the StreamTask.

We need the TaskClassLoader to load the user defined XXStreamTask class (not the StreamTask
interface class) so that any other classes referenced inside the user defined task will also
be auto-loaded by this TaskClassLoader.


> On June 17, 2015, 9:03 p.m., Yan Fang wrote:
> > samza-core/src/main/scala/org/apache/samza/container/TaskInstance.scala, lines 103-111
> > <https://reviews.apache.org/r/35397/diff/2/?file=985837#file985837line103>
> >
> >     a little concernted about this. This means we will load the class every time
a message comes. Is this too much? 
> >     
> >     My suggestion is to put this code in RunLoop.runs, before the loop starts. What
do you think?

A class will only be loaded once, so I think the overhead should be minimal.

My concern for putting this code in RunLoop is that some of the logic in RunLoop like consumerMultiplexer.choose
are platform code and any of its referenced classes should not be loaded by the task class
loader.


- Guozhang


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


On June 16, 2015, 5:22 p.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35397/
> -----------------------------------------------------------
> 
> (Updated June 16, 2015, 5:22 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Bugs: SAMZA-697
>     https://issues.apache.org/jira/browse/SAMZA-697
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> Use a separate post-delegate classloader for user-defined tasks
> 
> 
> Diffs
> -----
> 
>   bin/check-all.sh 0725b82ed4f70b155f8bfe65cc6938d4647142d0 
>   docs/learn/documentation/versioned/jobs/logging.md 1d13d151316bd51c7a3730e40450000433b7968e

>   samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/config/TaskConfig.scala 0b3a235b5ab1d6bd60669bfe6023f6b0b4e943d3

>   samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala cbacd183420e9d1d72b05693b55a8f0a62d59fc5

>   samza-core/src/main/scala/org/apache/samza/container/TaskInstance.scala c5a5ea5dea9a950fc741625238f5bf8b1f362180

>   samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala 1c178a661e449c6bdfc4ce431aef9bb2d261a6c2

>   samza-core/src/main/scala/org/apache/samza/job/local/ProcessJobFactory.scala 4fac154709d72ab594485dad93c912b55fb1617e

>   samza-core/src/test/java/org/apache/samza/task/TestTaskClassLoader.java PRE-CREATION

>   samza-core/src/test/scala/org/apache/samza/container/TestSamzaContainer.scala 9fb1aa98fcd14397e8a4cb00c67537482e95fa53

>   samza-core/src/test/scala/org/apache/samza/container/TestTaskInstance.scala 7caad28c9298485753ab861da76793cf925953ed

> 
> Diff: https://reviews.apache.org/r/35397/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>


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