You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Felix Albani <fe...@gmail.com> on 2018/05/14 20:10:30 UTC
Review Request 67117: RANGER-2078
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67117/
-----------------------------------------------------------
Review request for ranger.
Bugs: RANGER-2078
https://issues.apache.org/jira/browse/RANGER-2078
Repository: ranger
Description
-------
RANGER-2078
Diffs
-----
tagsync/src/main/java/org/apache/ranger/tagsync/model/AbstractTagSource.java da4c5cbd3
tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSink.java b5dcc7526
tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSource.java eb7981411
tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSyncThreadListener.java PRE-CREATION
tagsync/src/main/java/org/apache/ranger/tagsync/process/TagSyncConfig.java 6d27b02cd
tagsync/src/main/java/org/apache/ranger/tagsync/process/TagSynchronizer.java 49ff76fb2
tagsync/src/main/java/org/apache/ranger/tagsync/sink/tagadmin/TagAdminRESTSink.java 331f783db
tagsync/src/main/java/org/apache/ranger/tagsync/source/atlas/AtlasTagSource.java ea4c20cc5
tagsync/src/main/java/org/apache/ranger/tagsync/source/atlasrest/AtlasRESTTagSource.java 2b4a668b8
tagsync/src/main/java/org/apache/ranger/tagsync/source/file/FileTagSource.java f0a3fd003
Diff: https://reviews.apache.org/r/67117/diff/1/
Testing
-------
Only been tested in local cluster. Haven't created tests for this as I'm not sure how to simulate a RuntimeException.
Thanks,
Felix Albani
Re: Review Request 67117: RANGER-2078
Posted by Felix Albani <fe...@gmail.com>.
> On May 25, 2018, 10:50 p.m., Abhay Kulkarni wrote:
> > Felix - thanks for the patch to address this issue. A simpler solution for this issue (of Kafka notificaiton thread aborting) has alreay been committed via RANGER-2104 - in ranger-0.7 (https://reviews.apache.org/r/67160/), ranger-1.0 and master branches. Please review the details of this fix and let me know if you have further comments or suggestions.
> >
> > Please note that this issue is relevant only for Kafka notification listener thread; threads that handle REST call interface and file interface are not impacted.
> >
> > Thanks!
Good to know we have a solution for the Kafka notification thread aborting. Thanks!
- Felix
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67117/#review203918
-----------------------------------------------------------
On May 25, 2018, 6:07 p.m., Felix Albani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67117/
> -----------------------------------------------------------
>
> (Updated May 25, 2018, 6:07 p.m.)
>
>
> Review request for ranger.
>
>
> Bugs: RANGER-2078
> https://issues.apache.org/jira/browse/RANGER-2078
>
>
> Repository: ranger
>
>
> Description
> -------
>
> RANGER-2078
>
>
> Diffs
> -----
>
> tagsync/src/main/java/org/apache/ranger/tagsync/model/AbstractTagSource.java da4c5cbd3
> tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSink.java b5dcc7526
> tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSource.java eb7981411
> tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSyncThreadListener.java PRE-CREATION
> tagsync/src/main/java/org/apache/ranger/tagsync/process/TagSyncConfig.java 6d27b02cd
> tagsync/src/main/java/org/apache/ranger/tagsync/process/TagSynchronizer.java 49ff76fb2
> tagsync/src/main/java/org/apache/ranger/tagsync/sink/tagadmin/TagAdminRESTSink.java 331f783db
> tagsync/src/main/java/org/apache/ranger/tagsync/source/atlas/AtlasTagSource.java ea4c20cc5
> tagsync/src/main/java/org/apache/ranger/tagsync/source/atlasrest/AtlasRESTTagSource.java 2b4a668b8
> tagsync/src/main/java/org/apache/ranger/tagsync/source/file/FileTagSource.java f0a3fd003
>
>
> Diff: https://reviews.apache.org/r/67117/diff/1/
>
>
> Testing
> -------
>
> Only been tested in local cluster. Haven't created tests for this as I'm not sure how to simulate a RuntimeException.
>
>
> File Attachments
> ----------------
>
> new diff
> https://reviews.apache.org/media/uploaded/files/2018/05/25/51cc47ef-530c-4593-afcc-d3ee886c9888__0001-RANGER-2078-TagSync-process-is-running-but-not-doing-2.patch
>
>
> Thanks,
>
> Felix Albani
>
>
Re: Review Request 67117: RANGER-2078
Posted by Abhay Kulkarni <ak...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67117/#review203918
-----------------------------------------------------------
Felix - thanks for the patch to address this issue. A simpler solution for this issue (of Kafka notificaiton thread aborting) has alreay been committed via RANGER-2104 - in ranger-0.7 (https://reviews.apache.org/r/67160/), ranger-1.0 and master branches. Please review the details of this fix and let me know if you have further comments or suggestions.
Please note that this issue is relevant only for Kafka notification listener thread; threads that handle REST call interface and file interface are not impacted.
Thanks!
- Abhay Kulkarni
On May 25, 2018, 6:07 p.m., Felix Albani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67117/
> -----------------------------------------------------------
>
> (Updated May 25, 2018, 6:07 p.m.)
>
>
> Review request for ranger.
>
>
> Bugs: RANGER-2078
> https://issues.apache.org/jira/browse/RANGER-2078
>
>
> Repository: ranger
>
>
> Description
> -------
>
> RANGER-2078
>
>
> Diffs
> -----
>
> tagsync/src/main/java/org/apache/ranger/tagsync/model/AbstractTagSource.java da4c5cbd3
> tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSink.java b5dcc7526
> tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSource.java eb7981411
> tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSyncThreadListener.java PRE-CREATION
> tagsync/src/main/java/org/apache/ranger/tagsync/process/TagSyncConfig.java 6d27b02cd
> tagsync/src/main/java/org/apache/ranger/tagsync/process/TagSynchronizer.java 49ff76fb2
> tagsync/src/main/java/org/apache/ranger/tagsync/sink/tagadmin/TagAdminRESTSink.java 331f783db
> tagsync/src/main/java/org/apache/ranger/tagsync/source/atlas/AtlasTagSource.java ea4c20cc5
> tagsync/src/main/java/org/apache/ranger/tagsync/source/atlasrest/AtlasRESTTagSource.java 2b4a668b8
> tagsync/src/main/java/org/apache/ranger/tagsync/source/file/FileTagSource.java f0a3fd003
>
>
> Diff: https://reviews.apache.org/r/67117/diff/1/
>
>
> Testing
> -------
>
> Only been tested in local cluster. Haven't created tests for this as I'm not sure how to simulate a RuntimeException.
>
>
> File Attachments
> ----------------
>
> new diff
> https://reviews.apache.org/media/uploaded/files/2018/05/25/51cc47ef-530c-4593-afcc-d3ee886c9888__0001-RANGER-2078-TagSync-process-is-running-but-not-doing-2.patch
>
>
> Thanks,
>
> Felix Albani
>
>
Re: Review Request 67117: RANGER-2078
Posted by Felix Albani <fe...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67117/
-----------------------------------------------------------
(Updated May 25, 2018, 6:07 p.m.)
Review request for ranger.
Changes
-------
Im not able to update the diff somehow I get different errors. Please advise.
Bugs: RANGER-2078
https://issues.apache.org/jira/browse/RANGER-2078
Repository: ranger
Description
-------
RANGER-2078
Diffs
-----
tagsync/src/main/java/org/apache/ranger/tagsync/model/AbstractTagSource.java da4c5cbd3
tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSink.java b5dcc7526
tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSource.java eb7981411
tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSyncThreadListener.java PRE-CREATION
tagsync/src/main/java/org/apache/ranger/tagsync/process/TagSyncConfig.java 6d27b02cd
tagsync/src/main/java/org/apache/ranger/tagsync/process/TagSynchronizer.java 49ff76fb2
tagsync/src/main/java/org/apache/ranger/tagsync/sink/tagadmin/TagAdminRESTSink.java 331f783db
tagsync/src/main/java/org/apache/ranger/tagsync/source/atlas/AtlasTagSource.java ea4c20cc5
tagsync/src/main/java/org/apache/ranger/tagsync/source/atlasrest/AtlasRESTTagSource.java 2b4a668b8
tagsync/src/main/java/org/apache/ranger/tagsync/source/file/FileTagSource.java f0a3fd003
Diff: https://reviews.apache.org/r/67117/diff/1/
Testing
-------
Only been tested in local cluster. Haven't created tests for this as I'm not sure how to simulate a RuntimeException.
File Attachments (updated)
----------------
new diff
https://reviews.apache.org/media/uploaded/files/2018/05/25/51cc47ef-530c-4593-afcc-d3ee886c9888__0001-RANGER-2078-TagSync-process-is-running-but-not-doing-2.patch
Thanks,
Felix Albani
Re: Review Request 67117: RANGER-2078
Posted by Felix Albani <fe...@gmail.com>.
> On May 14, 2018, 8:33 p.m., Zsombor Gegesy wrote:
> > tagsync/src/main/java/org/apache/ranger/tagsync/process/TagSynchronizer.java
> > Lines 315 (patched)
> > <https://reviews.apache.org/r/67117/diff/1/?file=2022153#file2022153line318>
> >
> > getTagSourceFromConfig(...) will create a new TagSource object. Why don't you put the original 'tagSourceToStop' into the 'failedTagSources' list?
I think is safer to create a new object. I'm thinking perhaps state of failed object may cause unknown issues (correct me if I'm wrong)
- Felix
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67117/#review203065
-----------------------------------------------------------
On May 14, 2018, 8:10 p.m., Felix Albani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67117/
> -----------------------------------------------------------
>
> (Updated May 14, 2018, 8:10 p.m.)
>
>
> Review request for ranger.
>
>
> Bugs: RANGER-2078
> https://issues.apache.org/jira/browse/RANGER-2078
>
>
> Repository: ranger
>
>
> Description
> -------
>
> RANGER-2078
>
>
> Diffs
> -----
>
> tagsync/src/main/java/org/apache/ranger/tagsync/model/AbstractTagSource.java da4c5cbd3
> tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSink.java b5dcc7526
> tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSource.java eb7981411
> tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSyncThreadListener.java PRE-CREATION
> tagsync/src/main/java/org/apache/ranger/tagsync/process/TagSyncConfig.java 6d27b02cd
> tagsync/src/main/java/org/apache/ranger/tagsync/process/TagSynchronizer.java 49ff76fb2
> tagsync/src/main/java/org/apache/ranger/tagsync/sink/tagadmin/TagAdminRESTSink.java 331f783db
> tagsync/src/main/java/org/apache/ranger/tagsync/source/atlas/AtlasTagSource.java ea4c20cc5
> tagsync/src/main/java/org/apache/ranger/tagsync/source/atlasrest/AtlasRESTTagSource.java 2b4a668b8
> tagsync/src/main/java/org/apache/ranger/tagsync/source/file/FileTagSource.java f0a3fd003
>
>
> Diff: https://reviews.apache.org/r/67117/diff/1/
>
>
> Testing
> -------
>
> Only been tested in local cluster. Haven't created tests for this as I'm not sure how to simulate a RuntimeException.
>
>
> Thanks,
>
> Felix Albani
>
>
Re: Review Request 67117: RANGER-2078
Posted by Zsombor Gegesy <zs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67117/#review203065
-----------------------------------------------------------
tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSyncThreadListener.java
Lines 23 (patched)
<https://reviews.apache.org/r/67117/#comment285116>
Passing "TagSource" instead of a name, would make the implementation simpler.
tagsync/src/main/java/org/apache/ranger/tagsync/process/TagSynchronizer.java
Line 48 (original), 50 (patched)
<https://reviews.apache.org/r/67117/#comment285117>
... so no name->TagSource mapping would needed
tagsync/src/main/java/org/apache/ranger/tagsync/process/TagSynchronizer.java
Lines 301 (patched)
<https://reviews.apache.org/r/67117/#comment285118>
... so the TagSource object will be passed as a method argument
tagsync/src/main/java/org/apache/ranger/tagsync/process/TagSynchronizer.java
Lines 315 (patched)
<https://reviews.apache.org/r/67117/#comment285119>
getTagSourceFromConfig(...) will create a new TagSource object. Why don't you put the original 'tagSourceToStop' into the 'failedTagSources' list?
tagsync/src/main/java/org/apache/ranger/tagsync/source/atlas/AtlasTagSource.java
Lines 57 (patched)
<https://reviews.apache.org/r/67117/#comment285120>
This new 'listeners' variable is not used in the class, as addListener/removeListener delegates to the ConsumerRunnable, so you don't need it.
- Zsombor Gegesy
On May 14, 2018, 8:10 p.m., Felix Albani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67117/
> -----------------------------------------------------------
>
> (Updated May 14, 2018, 8:10 p.m.)
>
>
> Review request for ranger.
>
>
> Bugs: RANGER-2078
> https://issues.apache.org/jira/browse/RANGER-2078
>
>
> Repository: ranger
>
>
> Description
> -------
>
> RANGER-2078
>
>
> Diffs
> -----
>
> tagsync/src/main/java/org/apache/ranger/tagsync/model/AbstractTagSource.java da4c5cbd3
> tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSink.java b5dcc7526
> tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSource.java eb7981411
> tagsync/src/main/java/org/apache/ranger/tagsync/model/TagSyncThreadListener.java PRE-CREATION
> tagsync/src/main/java/org/apache/ranger/tagsync/process/TagSyncConfig.java 6d27b02cd
> tagsync/src/main/java/org/apache/ranger/tagsync/process/TagSynchronizer.java 49ff76fb2
> tagsync/src/main/java/org/apache/ranger/tagsync/sink/tagadmin/TagAdminRESTSink.java 331f783db
> tagsync/src/main/java/org/apache/ranger/tagsync/source/atlas/AtlasTagSource.java ea4c20cc5
> tagsync/src/main/java/org/apache/ranger/tagsync/source/atlasrest/AtlasRESTTagSource.java 2b4a668b8
> tagsync/src/main/java/org/apache/ranger/tagsync/source/file/FileTagSource.java f0a3fd003
>
>
> Diff: https://reviews.apache.org/r/67117/diff/1/
>
>
> Testing
> -------
>
> Only been tested in local cluster. Haven't created tests for this as I'm not sure how to simulate a RuntimeException.
>
>
> Thanks,
>
> Felix Albani
>
>