You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Rajat Khandelwal <ra...@gmail.com> on 2016/05/31 10:50:23 UTC
Review Request 48071: LENS-1160: classloader getting closed on lens
session close.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/
-----------------------------------------------------------
Review request for lens.
Bugs: LENS-1160
https://issues.apache.org/jira/browse/LENS-1160
Repository: lens
Description
-------
Diffs
-----
lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 82a4e15a6e5b12fb5f8ac8fe43076942266d3db5
lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
Diff: https://reviews.apache.org/r/48071/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On June 2, 2016, 3:23 p.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java, line 273
> > <https://reviews.apache.org/r/48071/diff/4/?file=1403142#file1403142line273>
> >
> > We are returning a new classloader even when resources is empty? Seems this is making one more extra copy unnecessarily.
>
> Rajat Khandelwal wrote:
> It's a dummy instance, since java's classloading logic will ask parent classloader to load class first. So this is just a placeholder without taking extra space.
Plus, it's an improvement over current design. Current design copies all urls and makes a new classloader in case session jars are added. And this happens each time you switch database. Now, with parent loaders, classes loaded by parent loader won't be loaded again and returned fast.
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/#review135908
-----------------------------------------------------------
On June 2, 2016, 5:11 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48071/
> -----------------------------------------------------------
>
> (Updated June 2, 2016, 5:11 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1160
> https://issues.apache.org/jira/browse/LENS-1160
>
>
> Repository: lens
>
>
> Description
> -------
>
> Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
> lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java 6c5e52d150bdbbb1075d9150901068bbd3400594
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 2b84d3a3d93d120119e8866e53cc9aa71b75dafd
> lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
> lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
>
> Diff: https://reviews.apache.org/r/48071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On June 2, 2016, 3:23 p.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java, line 273
> > <https://reviews.apache.org/r/48071/diff/4/?file=1403142#file1403142line273>
> >
> > We are returning a new classloader even when resources is empty? Seems this is making one more extra copy unnecessarily.
It's a dummy instance, since java's classloading logic will ask parent classloader to load class first. So this is just a placeholder without taking extra space.
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/#review135908
-----------------------------------------------------------
On June 2, 2016, 3:16 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48071/
> -----------------------------------------------------------
>
> (Updated June 2, 2016, 3:16 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1160
> https://issues.apache.org/jira/browse/LENS-1160
>
>
> Repository: lens
>
>
> Description
> -------
>
> Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
> lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java 6c5e52d150bdbbb1075d9150901068bbd3400594
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 82a4e15a6e5b12fb5f8ac8fe43076942266d3db5
> lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
> lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
>
> Diff: https://reviews.apache.org/r/48071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/#review135908
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java (line 238)
<https://reviews.apache.org/r/48071/#comment200929>
We are returning a new classloader even when resources is empty? Seems this is making one more extra copy unnecessarily.
- Amareshwari Sriramadasu
On June 2, 2016, 9:46 a.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48071/
> -----------------------------------------------------------
>
> (Updated June 2, 2016, 9:46 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1160
> https://issues.apache.org/jira/browse/LENS-1160
>
>
> Repository: lens
>
>
> Description
> -------
>
> Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
> lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java 6c5e52d150bdbbb1075d9150901068bbd3400594
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 82a4e15a6e5b12fb5f8ac8fe43076942266d3db5
> lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
> lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
>
> Diff: https://reviews.apache.org/r/48071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On June 3, 2016, 10:36 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java, line 28
> > <https://reviews.apache.org/r/48071/diff/6/?file=1404624#file1404624line28>
> >
> > Why is it extending SessionClassLoader ? Doesnt look right as Session would be one per sessiona and UncloseableClassLoader is created one per DB.
Just needed the `isClosed` feature. I made do with 2 classes, but if it's counter-intuitive, I can make 3 classes for that.
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/#review136038
-----------------------------------------------------------
On June 2, 2016, 5:11 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48071/
> -----------------------------------------------------------
>
> (Updated June 2, 2016, 5:11 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1160
> https://issues.apache.org/jira/browse/LENS-1160
>
>
> Repository: lens
>
>
> Description
> -------
>
> Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
> lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java 6c5e52d150bdbbb1075d9150901068bbd3400594
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 2b84d3a3d93d120119e8866e53cc9aa71b75dafd
> lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
> lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
>
> Diff: https://reviews.apache.org/r/48071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/#review136038
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java (line 28)
<https://reviews.apache.org/r/48071/#comment201034>
Why is it extending SessionClassLoader ? Doesnt look right as Session would be one per sessiona and UncloseableClassLoader is created one per DB.
- Amareshwari Sriramadasu
On June 2, 2016, 11:41 a.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48071/
> -----------------------------------------------------------
>
> (Updated June 2, 2016, 11:41 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1160
> https://issues.apache.org/jira/browse/LENS-1160
>
>
> Repository: lens
>
>
> Description
> -------
>
> Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
> lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java 6c5e52d150bdbbb1075d9150901068bbd3400594
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 2b84d3a3d93d120119e8866e53cc9aa71b75dafd
> lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
> lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
>
> Diff: https://reviews.apache.org/r/48071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/
-----------------------------------------------------------
(Updated June 8, 2016, 9:10 p.m.)
Review request for lens.
Bugs: LENS-1160
https://issues.apache.org/jira/browse/LENS-1160
Repository: lens
Description
-------
Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
Diffs (updated)
-----
lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 2b84d3a3d93d120119e8866e53cc9aa71b75dafd
lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
Diff: https://reviews.apache.org/r/48071/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/#review136610
-----------------------------------------------------------
Ship it!
Looks fine to me.
- Amareshwari Sriramadasu
On June 8, 2016, 8:16 a.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48071/
> -----------------------------------------------------------
>
> (Updated June 8, 2016, 8:16 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1160
> https://issues.apache.org/jira/browse/LENS-1160
>
>
> Repository: lens
>
>
> Description
> -------
>
> Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 2b84d3a3d93d120119e8866e53cc9aa71b75dafd
> lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
> lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
>
> Diff: https://reviews.apache.org/r/48071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/
-----------------------------------------------------------
(Updated June 8, 2016, 1:46 p.m.)
Review request for lens.
Bugs: LENS-1160
https://issues.apache.org/jira/browse/LENS-1160
Repository: lens
Description
-------
Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
Diffs (updated)
-----
lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 2b84d3a3d93d120119e8866e53cc9aa71b75dafd
lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
Diff: https://reviews.apache.org/r/48071/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On June 8, 2016, 9:06 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java, line 359
> > <https://reviews.apache.org/r/48071/diff/8/?file=1408769#file1408769line359>
> >
> > I feel we should not doing this everytime - almost every acquire - seems costly.
> >
> > Can we put updatedClassLoader in addResource itself ?
>
> Rajat Khandelwal wrote:
> This is not happening every time. This will happen only when cache doesn't have entry for that database. And made some change with which the entire classloader cache will get updated on each add/delete resource. New entries to the cache, however, will come through here.
Checked further with adding breackpoint on this line. Checking the stack tracw whenever The break point is hit, and none of them came from `acquire`. Also added one more optimization making the flow the following:
1. On add resource, the url is added to all classloaders of the cache. This doesn't create new instances of classloaders.
2. On set current database, it's ensured that the cache has an entry for this database. This is to ensure that next acquire calls are fast at the expense of this set-db call.
3. On remove resource, all entries of the cache are rebuild from new set of urls. This is needed since remove url is not available in classloader class. Add is cheap, remove is expensive.
4. `acquire` never creates a class loader, as seen from test cases.
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/#review136583
-----------------------------------------------------------
On June 8, 2016, 1:46 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48071/
> -----------------------------------------------------------
>
> (Updated June 8, 2016, 1:46 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1160
> https://issues.apache.org/jira/browse/LENS-1160
>
>
> Repository: lens
>
>
> Description
> -------
>
> Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 2b84d3a3d93d120119e8866e53cc9aa71b75dafd
> lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
> lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
>
> Diff: https://reviews.apache.org/r/48071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On June 8, 2016, 9:06 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java, line 359
> > <https://reviews.apache.org/r/48071/diff/8/?file=1408769#file1408769line359>
> >
> > I feel we should not doing this everytime - almost every acquire - seems costly.
> >
> > Can we put updatedClassLoader in addResource itself ?
This is not happening every time. This will happen only when cache doesn't have entry for that database. And made some change with which the entire classloader cache will get updated on each add/delete resource. New entries to the cache, however, will come through here.
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/#review136583
-----------------------------------------------------------
On June 7, 2016, 6:23 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48071/
> -----------------------------------------------------------
>
> (Updated June 7, 2016, 6:23 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1160
> https://issues.apache.org/jira/browse/LENS-1160
>
>
> Repository: lens
>
>
> Description
> -------
>
> Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 2b84d3a3d93d120119e8866e53cc9aa71b75dafd
> lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
> lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
>
> Diff: https://reviews.apache.org/r/48071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/#review136583
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java (line 189)
<https://reviews.apache.org/r/48071/#comment201672>
should be private ?
lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java (line 356)
<https://reviews.apache.org/r/48071/#comment201673>
I feel we should not doing this everytime - almost every acquire - seems costly.
Can we put updatedClassLoader in addResource itself ?
- Amareshwari Sriramadasu
On June 7, 2016, 12:53 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48071/
> -----------------------------------------------------------
>
> (Updated June 7, 2016, 12:53 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1160
> https://issues.apache.org/jira/browse/LENS-1160
>
>
> Repository: lens
>
>
> Description
> -------
>
> Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 2b84d3a3d93d120119e8866e53cc9aa71b75dafd
> lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
> lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
>
> Diff: https://reviews.apache.org/r/48071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/
-----------------------------------------------------------
(Updated June 7, 2016, 6:23 p.m.)
Review request for lens.
Bugs: LENS-1160
https://issues.apache.org/jira/browse/LENS-1160
Repository: lens
Description
-------
Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
Diffs (updated)
-----
lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 2b84d3a3d93d120119e8866e53cc9aa71b75dafd
lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
Diff: https://reviews.apache.org/r/48071/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/
-----------------------------------------------------------
(Updated June 7, 2016, 5:21 p.m.)
Review request for lens.
Bugs: LENS-1160
https://issues.apache.org/jira/browse/LENS-1160
Repository: lens
Description
-------
Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
Diffs (updated)
-----
lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java 6c5e52d150bdbbb1075d9150901068bbd3400594
lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 2b84d3a3d93d120119e8866e53cc9aa71b75dafd
lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java fdc8fe0386db1379636b9bae64b6639b98b67ace
lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
Diff: https://reviews.apache.org/r/48071/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/
-----------------------------------------------------------
(Updated June 2, 2016, 5:11 p.m.)
Review request for lens.
Bugs: LENS-1160
https://issues.apache.org/jira/browse/LENS-1160
Repository: lens
Description
-------
Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
Diffs (updated)
-----
lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java 6c5e52d150bdbbb1075d9150901068bbd3400594
lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 2b84d3a3d93d120119e8866e53cc9aa71b75dafd
lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
Diff: https://reviews.apache.org/r/48071/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/
-----------------------------------------------------------
(Updated June 2, 2016, 3:16 p.m.)
Review request for lens.
Bugs: LENS-1160
https://issues.apache.org/jira/browse/LENS-1160
Repository: lens
Description
-------
Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
Diffs (updated)
-----
lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java 6c5e52d150bdbbb1075d9150901068bbd3400594
lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 82a4e15a6e5b12fb5f8ac8fe43076942266d3db5
lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
Diff: https://reviews.apache.org/r/48071/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/
-----------------------------------------------------------
(Updated June 1, 2016, 6:52 p.m.)
Review request for lens.
Bugs: LENS-1160
https://issues.apache.org/jira/browse/LENS-1160
Repository: lens
Description
-------
Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
Diffs (updated)
-----
lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java 6c5e52d150bdbbb1075d9150901068bbd3400594
lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 82a4e15a6e5b12fb5f8ac8fe43076942266d3db5
lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
Diff: https://reviews.apache.org/r/48071/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/
-----------------------------------------------------------
(Updated June 1, 2016, 6:04 p.m.)
Review request for lens.
Bugs: LENS-1160
https://issues.apache.org/jira/browse/LENS-1160
Repository: lens
Description
-------
Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
Diffs (updated)
-----
lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java 6c5e52d150bdbbb1075d9150901068bbd3400594
lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 82a4e15a6e5b12fb5f8ac8fe43076942266d3db5
lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
Diff: https://reviews.apache.org/r/48071/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On June 1, 2016, 10:44 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java, line 233
> > <https://reviews.apache.org/r/48071/diff/2/?file=1402068#file1402068line233>
> >
> > Why are we adding these public acquire and relaase with overriden implemenations? I dont see any callers for them
So if someone has a `LensSessionImpl` instance and calls `acquire(true)`, it'll actually call `HiveSessionImpl.acquire`. I wanted polymorphism to hold true in all cases. Hence added these.
> On June 1, 2016, 10:44 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java, line 51
> > <https://reviews.apache.org/r/48071/diff/2/?file=1402066#file1402066line51>
> >
> > Why are we extending UDFClassLoader? Does not seem to be correct classloader to extend. The doc on UDFClassLoader says thats meant one per session.
>
> Rajat Khandelwal wrote:
> UDFClassLoader is basically URLClassLoader plus extra boolean indicating whether it's closed or not. The information `isClosed()` seems valuable for test cases.
>
> Amareshwari Sriramadasu wrote:
> I would say lets not have this dependency with UDFClassLoader just to use isClosed() method - as hive could behave differently when classloader is UDFClassLoader instance
Created new classloader classes.
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/#review135765
-----------------------------------------------------------
On June 1, 2016, 6:04 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48071/
> -----------------------------------------------------------
>
> (Updated June 1, 2016, 6:04 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1160
> https://issues.apache.org/jira/browse/LENS-1160
>
>
> Repository: lens
>
>
> Description
> -------
>
> Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
> lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java 6c5e52d150bdbbb1075d9150901068bbd3400594
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 82a4e15a6e5b12fb5f8ac8fe43076942266d3db5
> lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java PRE-CREATION
> lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java PRE-CREATION
> lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
>
> Diff: https://reviews.apache.org/r/48071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Amareshwari Sriramadasu <am...@apache.org>.
> On June 1, 2016, 5:14 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java, line 51
> > <https://reviews.apache.org/r/48071/diff/2/?file=1402066#file1402066line51>
> >
> > Why are we extending UDFClassLoader? Does not seem to be correct classloader to extend. The doc on UDFClassLoader says thats meant one per session.
>
> Rajat Khandelwal wrote:
> UDFClassLoader is basically URLClassLoader plus extra boolean indicating whether it's closed or not. The information `isClosed()` seems valuable for test cases.
I would say lets not have this dependency with UDFClassLoader just to use isClosed() method - as hive could behave differently when classloader is UDFClassLoader instance
- Amareshwari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/#review135765
-----------------------------------------------------------
On May 31, 2016, 2:35 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48071/
> -----------------------------------------------------------
>
> (Updated May 31, 2016, 2:35 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1160
> https://issues.apache.org/jira/browse/LENS-1160
>
>
> Repository: lens
>
>
> Description
> -------
>
> Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
> lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java 6c5e52d150bdbbb1075d9150901068bbd3400594
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 82a4e15a6e5b12fb5f8ac8fe43076942266d3db5
> lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
>
> Diff: https://reviews.apache.org/r/48071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On June 1, 2016, 10:44 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java, line 51
> > <https://reviews.apache.org/r/48071/diff/2/?file=1402066#file1402066line51>
> >
> > Why are we extending UDFClassLoader? Does not seem to be correct classloader to extend. The doc on UDFClassLoader says thats meant one per session.
UDFClassLoader is basically URLClassLoader plus extra boolean indicating whether it's closed or not. The information `isClosed()` seems valuable for test cases.
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/#review135765
-----------------------------------------------------------
On May 31, 2016, 8:05 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48071/
> -----------------------------------------------------------
>
> (Updated May 31, 2016, 8:05 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1160
> https://issues.apache.org/jira/browse/LENS-1160
>
>
> Repository: lens
>
>
> Description
> -------
>
> Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
> lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java 6c5e52d150bdbbb1075d9150901068bbd3400594
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 82a4e15a6e5b12fb5f8ac8fe43076942266d3db5
> lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
>
> Diff: https://reviews.apache.org/r/48071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/#review135765
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java (line 51)
<https://reviews.apache.org/r/48071/#comment200814>
Why are we extending UDFClassLoader? Does not seem to be correct classloader to extend. The doc on UDFClassLoader says thats meant one per session.
lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java (line 254)
<https://reviews.apache.org/r/48071/#comment200815>
why are we creating a new UDFClassloader ?
lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java (line 196)
<https://reviews.apache.org/r/48071/#comment200816>
Can you add comment wrt whats getting closed here ?
lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java (line 229)
<https://reviews.apache.org/r/48071/#comment200817>
Why are we adding these public acquire and relaase with overriden implemenations? I dont see any callers for them
lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java (line 300)
<https://reviews.apache.org/r/48071/#comment200818>
Can we update tests with class.forName calls for resouce classes?
lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java (line 305)
<https://reviews.apache.org/r/48071/#comment200819>
Can we have a test which opens second session after first one is closed?
- Amareshwari Sriramadasu
On May 31, 2016, 2:35 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48071/
> -----------------------------------------------------------
>
> (Updated May 31, 2016, 2:35 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1160
> https://issues.apache.org/jira/browse/LENS-1160
>
>
> Repository: lens
>
>
> Description
> -------
>
> Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
> lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java 6c5e52d150bdbbb1075d9150901068bbd3400594
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 82a4e15a6e5b12fb5f8ac8fe43076942266d3db5
> lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
>
> Diff: https://reviews.apache.org/r/48071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Puneet Gupta <pu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/#review135748
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java (line 51)
<https://reviews.apache.org/r/48071/#comment200806>
Should we extend URL class loader instead and call it UncloseableURLClassLoader ?
- Puneet Gupta
On May 31, 2016, 2:35 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48071/
> -----------------------------------------------------------
>
> (Updated May 31, 2016, 2:35 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1160
> https://issues.apache.org/jira/browse/LENS-1160
>
>
> Repository: lens
>
>
> Description
> -------
>
> Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
> lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java 6c5e52d150bdbbb1075d9150901068bbd3400594
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 82a4e15a6e5b12fb5f8ac8fe43076942266d3db5
> lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
>
> Diff: https://reviews.apache.org/r/48071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/
-----------------------------------------------------------
(Updated May 31, 2016, 8:05 p.m.)
Review request for lens.
Bugs: LENS-1160
https://issues.apache.org/jira/browse/LENS-1160
Repository: lens
Description (updated)
-------
Creating an UncloseableClassloader in DatabaseResourceService. Each db has a class loader which will be uncloseable. Lens session asks db service for a new classloader, which has the uncloseable classloader as parent. On session close, Lens session is closing its individual class loaders per db, but will leave the db class loader intact. This way we are making sure the existing design remains intact. Each session sharing a common database (without session jars) will have a common Uncloseable class loader. A session will have classloaders per db, which will the db class loader (uncloseable) in case no extra jars have been added for that db in that session, or it will be a UDFClassLoader with the uncloseable db classloader as parent. This ensures that classes already loaded by db class loader will not be loaded multiple times by each session. Also, the lifecycle of db resource service starts with lens server and ends with it too. Which means, those classloaders never need to close.
Diffs (updated)
-----
lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java 6c5e52d150bdbbb1075d9150901068bbd3400594
lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 82a4e15a6e5b12fb5f8ac8fe43076942266d3db5
lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
Diff: https://reviews.apache.org/r/48071/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 48071: LENS-1160: classloader getting closed on
lens session close.
Posted by Puneet Gupta <pu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48071/#review135634
-----------------------------------------------------------
lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java (line 298)
<https://reviews.apache.org/r/48071/#comment200658>
Can we find a way to re-use calss loaders. If possible we should create a new classloader with this one as parent. This will make sure we are not loading the same class again in multiple classloaders.
- Puneet Gupta
On May 31, 2016, 10:50 a.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48071/
> -----------------------------------------------------------
>
> (Updated May 31, 2016, 10:50 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1160
> https://issues.apache.org/jira/browse/LENS-1160
>
>
> Repository: lens
>
>
> Description
> -------
>
>
> Diffs
> -----
>
> lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java 1f63ed780552f921315ddee49d61495716e57227
> lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java 82a4e15a6e5b12fb5f8ac8fe43076942266d3db5
> lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java eeb97f2cbb5257acdacddea7048d1e806088e5cc
>
> Diff: https://reviews.apache.org/r/48071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>