You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Guozhang Wang <wa...@gmail.com> on 2015/06/12 19:57:09 UTC
Review Request 35397: Fix SAMZA-697
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35397/
-----------------------------------------------------------
Review request for samza.
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/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
-------
This is WIP patch, I need to add some unit tests for this change. But I would like to have people taking a look at it now to make sure I am on the right direction.
Thanks,
Guozhang Wang
Re: Review Request 35397: Fix SAMZA-697
Posted by Guozhang Wang <wa...@gmail.com>.
> 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?
>
> Guozhang Wang wrote:
> 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.
>
> Yan Fang wrote:
> Oh, I guess my concern is more about the time spent in "getContextClassLoader", "setContextClassLoader", etc. They actually are useless after loading the class but called every time a message comes. Maybe we want to optimize it a little.
>
> Another question is, why do we need to have taskClassLoader here if we already use it to instantiate the XXXStreamTask, which should reslove all the dependencies in the XXXStreamTask? Guess I have some misunderstanding in the classLoader.
After a second thought, there seems no need to set / get context classloader any more since we already explicitly use the task class loader for loading the enclosing class XXXTask. Will change that.
- Guozhang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35397/#review88260
-----------------------------------------------------------
On June 18, 2015, 6:42 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35397/
> -----------------------------------------------------------
>
> (Updated June 18, 2015, 6:42 p.m.)
>
>
> Review request for samza.
>
>
> Bugs: SAMZA-697
> https://issues.apache.org/jira/browse/SAMZA-697
>
>
> Repository: samza
>
>
> Description
> -------
>
> Address Yan's comments
>
>
> Diffs
> -----
>
> checkstyle/import-control.xml 3374f0c432e61ac4cda275377604cfd481f0cddf
> docs/learn/documentation/versioned/jobs/configuration-table.html 405e2cea4fd1d037cc26b3537f6bb406eded202b
> 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
>
>
Re: Review Request 35397: Fix SAMZA-697
Posted by Yan Fang <ya...@gmail.com>.
> 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?
>
> Guozhang Wang wrote:
> 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.
Oh, I guess my concern is more about the time spent in "getContextClassLoader", "setContextClassLoader", etc. They actually are useless after loading the class but called every time a message comes. Maybe we want to optimize it a little.
Another question is, why do we need to have taskClassLoader here if we already use it to instantiate the XXXStreamTask, which should reslove all the dependencies in the XXXStreamTask? Guess I have some misunderstanding in the classLoader.
- Yan
-----------------------------------------------------------
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
>
>
Re: Review Request 35397: Fix SAMZA-697
Posted by Guozhang Wang <wa...@gmail.com>.
> 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
>
>
Re: Review Request 35397: Fix SAMZA-697
Posted by Yan Fang <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35397/#review88260
-----------------------------------------------------------
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 44)
<https://reviews.apache.org/r/35397/#comment140682>
change the scope?
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 74)
<https://reviews.apache.org/r/35397/#comment140715>
also canonicalize the classNames in the blacklistClassnames list?
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 75)
<https://reviews.apache.org/r/35397/#comment140684>
use "if" to be consistent?
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 115)
<https://reviews.apache.org/r/35397/#comment140685>
to be "if"? Just my $0.02
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 156)
<https://reviews.apache.org/r/35397/#comment140704>
log here
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (lines 162 - 163)
<https://reviews.apache.org/r/35397/#comment140705>
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.
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 164)
<https://reviews.apache.org/r/35397/#comment140706>
you mean, parent.loadClass(name), right?
samza-core/src/main/scala/org/apache/samza/config/TaskConfig.scala (lines 40 - 42)
<https://reviews.apache.org/r/35397/#comment140717>
add those to configuration table as well, including what kind of format we accept.
samza-core/src/main/scala/org/apache/samza/config/TaskConfig.scala (line 112)
<https://reviews.apache.org/r/35397/#comment140707>
remove the space
samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala (lines 425 - 427)
<https://reviews.apache.org/r/35397/#comment140708>
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.
samza-core/src/main/scala/org/apache/samza/container/TaskInstance.scala (lines 97 - 105)
<https://reviews.apache.org/r/35397/#comment140709>
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?
samza-core/src/test/java/org/apache/samza/task/TestTaskClassLoader.java (line 88)
<https://reviews.apache.org/r/35397/#comment140716>
also want to test "org/example/foo" format
- Yan Fang
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
>
>
Re: Review Request 35397: Fix SAMZA-697
Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35397/
-----------------------------------------------------------
(Updated July 10, 2015, 4:54 p.m.)
Review request for samza.
Summary (updated)
-----------------
Fix SAMZA-697
Bugs: SAMZA-697
https://issues.apache.org/jira/browse/SAMZA-697
Repository: samza
Description (updated)
-------
Add missing files
Diffs (updated)
-----
checkstyle/import-control.xml 3374f0c432e61ac4cda275377604cfd481f0cddf
docs/learn/documentation/versioned/jobs/configuration-table.html 405e2cea4fd1d037cc26b3537f6bb406eded202b
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
Re: Review Request 35397: Fix SAMZA-697
Posted by Guozhang Wang <wa...@gmail.com>.
> On July 7, 2015, 12:42 a.m., Boris Shkolnik wrote:
> > The latest diff seems incomplete. Missing some file(s).
Ahh yes. Added the missing files.
- Guozhang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35397/#review90620
-----------------------------------------------------------
On July 10, 2015, 4:54 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35397/
> -----------------------------------------------------------
>
> (Updated July 10, 2015, 4:54 p.m.)
>
>
> Review request for samza.
>
>
> Bugs: SAMZA-697
> https://issues.apache.org/jira/browse/SAMZA-697
>
>
> Repository: samza
>
>
> Description
> -------
>
> Add missing files
>
>
> Diffs
> -----
>
> checkstyle/import-control.xml 3374f0c432e61ac4cda275377604cfd481f0cddf
> docs/learn/documentation/versioned/jobs/configuration-table.html 405e2cea4fd1d037cc26b3537f6bb406eded202b
> 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
>
>
Re: Review Request 35397: Fix Samza-697
Posted by Boris Shkolnik <bo...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35397/#review90620
-----------------------------------------------------------
The latest diff seems incomplete. Missing some file(s).
- Boris Shkolnik
On June 30, 2015, 6:29 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35397/
> -----------------------------------------------------------
>
> (Updated June 30, 2015, 6:29 p.m.)
>
>
> Review request for samza.
>
>
> Bugs: SAMZA-697
> https://issues.apache.org/jira/browse/SAMZA-697
>
>
> Repository: samza
>
>
> Description
> -------
>
> Address Boris and Yi's comments
>
>
> Diffs
> -----
>
> checkstyle/import-control.xml 3374f0c432e61ac4cda275377604cfd481f0cddf
> docs/learn/documentation/versioned/jobs/configuration-table.html 405e2cea4fd1d037cc26b3537f6bb406eded202b
> 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/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
>
>
Re: Review Request 35397: Fix Samza-697
Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35397/
-----------------------------------------------------------
(Updated June 30, 2015, 6:29 p.m.)
Review request for samza.
Summary (updated)
-----------------
Fix Samza-697
Bugs: SAMZA-697
https://issues.apache.org/jira/browse/SAMZA-697
Repository: samza
Description
-------
Address Boris and Yi's comments
Diffs
-----
checkstyle/import-control.xml 3374f0c432e61ac4cda275377604cfd481f0cddf
docs/learn/documentation/versioned/jobs/configuration-table.html 405e2cea4fd1d037cc26b3537f6bb406eded202b
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/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
Re: Review Request 35397: v1
Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35397/
-----------------------------------------------------------
(Updated June 30, 2015, 6:28 p.m.)
Review request for samza.
Summary (updated)
-----------------
v1
Bugs: SAMZA-697
https://issues.apache.org/jira/browse/SAMZA-697
Repository: samza
Description (updated)
-------
Address Boris and Yi's comments
Diffs (updated)
-----
checkstyle/import-control.xml 3374f0c432e61ac4cda275377604cfd481f0cddf
docs/learn/documentation/versioned/jobs/configuration-table.html 405e2cea4fd1d037cc26b3537f6bb406eded202b
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/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
Re: Review Request 35397: Fix SAMZA-697
Posted by Guozhang Wang <wa...@gmail.com>.
> On June 19, 2015, 8:59 a.m., Yi Pan (Data Infrastructure) wrote:
> > docs/learn/documentation/versioned/jobs/configuration-table.html, line 443
> > <https://reviews.apache.org/r/35397/diff/3/?file=987153#file987153line443>
> >
> > Users who extends and implements StreamTask usually have their implementation classes put in the same package as org.apache.samza.task. Wouldn't this default blacklist also ignore the user-implemented StreamTask classes?
Good point. I would expect though that if we cleanup the packaging in the future as suggested in building the import checklist, we will still blacklist "not samza.api." libraries.
> On June 19, 2015, 8:59 a.m., Yi Pan (Data Infrastructure) wrote:
> > samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java, line 156
> > <https://reviews.apache.org/r/35397/diff/3/?file=987154#file987154line156>
> >
> > Question: does this findLoadedClass(name) return true if the same binary name of a class has been loaded by another classloader? Can we add a test here to make sure?
findLoadedClass() does not delegate to parent, it "returns the class with the given the binary name if this loader has been recorded by the Java virtual machine as an initiating loader of a class" as quoted in java doc.
> On June 19, 2015, 8:59 a.m., Yi Pan (Data Infrastructure) wrote:
> > samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java, line 71
> > <https://reviews.apache.org/r/35397/diff/3/?file=987154#file987154line71>
> >
> > nit: for unit test, changing it to package default should work.
Not sure I understand clearly, could you elaborate how changing to package default can help accessing a private class function?
- Guozhang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35397/#review88236
-----------------------------------------------------------
On June 18, 2015, 6:42 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35397/
> -----------------------------------------------------------
>
> (Updated June 18, 2015, 6:42 p.m.)
>
>
> Review request for samza.
>
>
> Bugs: SAMZA-697
> https://issues.apache.org/jira/browse/SAMZA-697
>
>
> Repository: samza
>
>
> Description
> -------
>
> Address Yan's comments
>
>
> Diffs
> -----
>
> checkstyle/import-control.xml 3374f0c432e61ac4cda275377604cfd481f0cddf
> docs/learn/documentation/versioned/jobs/configuration-table.html 405e2cea4fd1d037cc26b3537f6bb406eded202b
> 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
>
>
Re: Review Request 35397: Fix SAMZA-697
Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35397/#review88236
-----------------------------------------------------------
Overall looks good. I have a few comments/questions. Thanks!
bin/check-all.sh (line 60)
<https://reviews.apache.org/r/35397/#comment140653>
I think that this is from another commit? You may need to rebase your changes.
docs/learn/documentation/versioned/jobs/configuration-table.html (line 443)
<https://reviews.apache.org/r/35397/#comment141085>
Users who extends and implements StreamTask usually have their implementation classes put in the same package as org.apache.samza.task. Wouldn't this default blacklist also ignore the user-implemented StreamTask classes?
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 71)
<https://reviews.apache.org/r/35397/#comment141086>
nit: for unit test, changing it to package default should work.
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 156)
<https://reviews.apache.org/r/35397/#comment141087>
Question: does this findLoadedClass(name) return true if the same binary name of a class has been loaded by another classloader? Can we add a test here to make sure?
samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala (line 433)
<https://reviews.apache.org/r/35397/#comment141089>
What if the taskClassLoaderPath is null? Shouldn't we just skip the swaping of classloaders?
Maybe it would be good to print out a warning now and use the default classloader. Then later enforcing it by throwing exception if taskClassLoaderPath is not setup.
samza-core/src/main/scala/org/apache/samza/container/TaskInstance.scala (line 97)
<https://reviews.apache.org/r/35397/#comment141088>
It would seem cleaner as:
def swapClassloader(userTask: => Unit) {
val thread = Thread.currentThread
val oldClassLoader = thread.getContextClassLoader
thread.setContextClassLoader(taskClassLoader)
try {
userTask
} finally {
thread.setContextClassLoader(oldClassLoader)
}
}
Then:
swapClassloader {
task.asInstanceOf[InitableTask].init(config, context)
}
swapClassloader {
exceptionHandler.maybeHandle {
task.process(envelope, collector, coordinator)
}
}
...
- Yi Pan (Data Infrastructure)
On June 18, 2015, 6:42 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35397/
> -----------------------------------------------------------
>
> (Updated June 18, 2015, 6:42 p.m.)
>
>
> Review request for samza.
>
>
> Bugs: SAMZA-697
> https://issues.apache.org/jira/browse/SAMZA-697
>
>
> Repository: samza
>
>
> Description
> -------
>
> Address Yan's comments
>
>
> Diffs
> -----
>
> checkstyle/import-control.xml 3374f0c432e61ac4cda275377604cfd481f0cddf
> docs/learn/documentation/versioned/jobs/configuration-table.html 405e2cea4fd1d037cc26b3537f6bb406eded202b
> 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
>
>
Re: Review Request 35397: Fix SAMZA-697
Posted by Guozhang Wang <wa...@gmail.com>.
> On June 18, 2015, 11:58 p.m., Boris Shkolnik wrote:
> > samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java, line 50
> > <https://reviews.apache.org/r/35397/diff/3/?file=987154#file987154line50>
> >
> > Do we have test for this case?
We check this case in TestTaskClassLoader.testConstructUrlsFromClasspath
> On June 18, 2015, 11:58 p.m., Boris Shkolnik wrote:
> > samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java, line 72
> > <https://reviews.apache.org/r/35397/diff/3/?file=987154#file987154line72>
> >
> > nit. may be if (blacklistClassnames == null) return false.
Wondering what is the difference for these two?
> On June 18, 2015, 11:58 p.m., Boris Shkolnik wrote:
> > samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java, line 140
> > <https://reviews.apache.org/r/35397/diff/3/?file=987154#file987154line140>
> >
> > else log an error?
If getResource returns null an exception will usually be thrown.
- Guozhang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35397/#review88455
-----------------------------------------------------------
On June 18, 2015, 6:42 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35397/
> -----------------------------------------------------------
>
> (Updated June 18, 2015, 6:42 p.m.)
>
>
> Review request for samza.
>
>
> Bugs: SAMZA-697
> https://issues.apache.org/jira/browse/SAMZA-697
>
>
> Repository: samza
>
>
> Description
> -------
>
> Address Yan's comments
>
>
> Diffs
> -----
>
> checkstyle/import-control.xml 3374f0c432e61ac4cda275377604cfd481f0cddf
> docs/learn/documentation/versioned/jobs/configuration-table.html 405e2cea4fd1d037cc26b3537f6bb406eded202b
> 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
>
>
Re: Review Request 35397: Fix SAMZA-697
Posted by Boris Shkolnik <bo...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35397/#review88455
-----------------------------------------------------------
This is partial review (I didn't go thru the test).
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 50)
<https://reviews.apache.org/r/35397/#comment140992>
Do we have test for this case?
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 63)
<https://reviews.apache.org/r/35397/#comment140989>
Do we need a LOG.warn in case the file doesn't exist.
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 72)
<https://reviews.apache.org/r/35397/#comment140993>
nit. may be if (blacklistClassnames == null) return false.
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 103)
<https://reviews.apache.org/r/35397/#comment140995>
should we have little bit more validation here (checking for empty strings for example)
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 109)
<https://reviews.apache.org/r/35397/#comment140996>
nit. should be checked at the beginning of the method.
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 129)
<https://reviews.apache.org/r/35397/#comment140997>
nit. 'blacklisted'
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 140)
<https://reviews.apache.org/r/35397/#comment140998>
else log an error?
samza-core/src/main/java/org/apache/samza/task/TaskClassLoader.java (line 146)
<https://reviews.apache.org/r/35397/#comment141000>
do we need to override it if it is the same a default implementation.
samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala (line 435)
<https://reviews.apache.org/r/35397/#comment141001>
Do we want to create this taskClassLoader if the taskClassLoaderPath is not configured? If this is the case we are creating classLoader with 'null' list of URLs. Is it safe?
This
- Boris Shkolnik
On June 18, 2015, 6:42 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35397/
> -----------------------------------------------------------
>
> (Updated June 18, 2015, 6:42 p.m.)
>
>
> Review request for samza.
>
>
> Bugs: SAMZA-697
> https://issues.apache.org/jira/browse/SAMZA-697
>
>
> Repository: samza
>
>
> Description
> -------
>
> Address Yan's comments
>
>
> Diffs
> -----
>
> checkstyle/import-control.xml 3374f0c432e61ac4cda275377604cfd481f0cddf
> docs/learn/documentation/versioned/jobs/configuration-table.html 405e2cea4fd1d037cc26b3537f6bb406eded202b
> 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
>
>
Re: Review Request 35397: Fix SAMZA-697
Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35397/
-----------------------------------------------------------
(Updated June 18, 2015, 6:42 p.m.)
Review request for samza.
Bugs: SAMZA-697
https://issues.apache.org/jira/browse/SAMZA-697
Repository: samza
Description (updated)
-------
Address Yan's comments
Diffs (updated)
-----
checkstyle/import-control.xml 3374f0c432e61ac4cda275377604cfd481f0cddf
docs/learn/documentation/versioned/jobs/configuration-table.html 405e2cea4fd1d037cc26b3537f6bb406eded202b
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
Re: Review Request 35397: Fix SAMZA-697
Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
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 (updated)
-----
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 (updated)
-------
unit tests
Thanks,
Guozhang Wang
Re: Review Request 35397: Fix SAMZA-697
Posted by Guozhang Wang <wa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35397/
-----------------------------------------------------------
(Updated June 12, 2015, 5:57 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/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
-------
This is WIP patch, I need to add some unit tests for this change. But I would like to have people taking a look at it now to make sure I am on the right direction.
Thanks,
Guozhang Wang