You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Ashutosh Mestry <am...@hortonworks.com> on 2018/09/07 19:00:30 UTC
Review Request 68671: Export Service: Incremental Export: Considers
RequestContext's Timestamp
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68671/
-----------------------------------------------------------
Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
Bugs: ATLAS-2862
https://issues.apache.org/jira/browse/ATLAS-2862
Repository: atlas
Description
-------
**Approach**
Using _RequestContextV1.earliestActiveRequestTime_.
Refactoring:
- Removed _AtlasExportData.entities_ since this had entire payload of exported entities, causing memory bloat.
- Changed _AtlasExport_ options' _fromTime_ to _changeToken_.
- Additional refactoring for better readability.
Diffs
-----
intg/src/main/java/org/apache/atlas/model/impexp/AtlasExportRequest.java 96a6e8855
intg/src/main/java/org/apache/atlas/model/impexp/AtlasExportResult.java fd6871219
intg/src/main/java/org/apache/atlas/model/impexp/AtlasImportResult.java f066688ea
repository/src/main/java/org/apache/atlas/repository/impexp/AuditsWriter.java 7fb2a3b61
repository/src/main/java/org/apache/atlas/repository/impexp/ExportService.java b507002bd
repository/src/main/java/org/apache/atlas/repository/impexp/ZipSink.java 17ebbf13e
repository/src/test/java/org/apache/atlas/repository/impexp/ExportIncrementalTest.java ed4fc372a
repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsShaperTest.java f89455318
repository/src/test/java/org/apache/atlas/repository/impexp/ReplicationEntityAttributeTest.java 719d6ca09
repository/src/test/java/org/apache/atlas/repository/impexp/ZipSinkTest.java e8bbeb590
repository/src/test/resources/json/stocksDB-Entities/export-incremental.json c2bc86757
Diff: https://reviews.apache.org/r/68671/diff/1/
Testing
-------
**Unit tests**
Existing tests.
**Volume tests**
Export with 150 tables, each with 80 columns data.
Thanks,
Ashutosh Mestry
Re: Review Request 68671: Export Service: Incremental Export:
Considers RequestContext's Timestamp
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68671/#review208476
-----------------------------------------------------------
Fix it, then Ship it!
repository/src/test/resources/json/stocksDB-Entities/export-incremental.json
Line 9 (original), 9 (patched)
<https://reviews.apache.org/r/68671/#comment292405>
Its 'changeMarker' in AtlasExportRequest.java. Please review and update here.
public static final String FETCH_TYPE_INCREMENTAL_CHANGE_MARKER = "changeMarker";
- Madhan Neethiraj
On Sept. 7, 2018, 10:14 p.m., Ashutosh Mestry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68671/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2018, 10:14 p.m.)
>
>
> Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-2862
> https://issues.apache.org/jira/browse/ATLAS-2862
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Approach**
> Using _RequestContextV1.earliestActiveRequestTime_.
>
> Refactoring:
> - Removed _AtlasExportData.entities_ since this had entire payload of exported entities, causing memory bloat.
> - Changed _AtlasExport_ options' _fromTime_ to _changeToken_.
> - Additional refactoring for better readability.
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/model/impexp/AtlasExportRequest.java 96a6e8855
> intg/src/main/java/org/apache/atlas/model/impexp/AtlasExportResult.java fd6871219
> intg/src/main/java/org/apache/atlas/model/impexp/AtlasImportResult.java f066688ea
> repository/src/main/java/org/apache/atlas/repository/impexp/AuditsWriter.java 7fb2a3b61
> repository/src/main/java/org/apache/atlas/repository/impexp/ExportService.java b507002bd
> repository/src/main/java/org/apache/atlas/repository/impexp/ZipSink.java 17ebbf13e
> repository/src/test/java/org/apache/atlas/repository/impexp/ExportIncrementalTest.java ed4fc372a
> repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsShaperTest.java f89455318
> repository/src/test/java/org/apache/atlas/repository/impexp/ReplicationEntityAttributeTest.java 719d6ca09
> repository/src/test/java/org/apache/atlas/repository/impexp/ZipSinkTest.java e8bbeb590
> repository/src/test/resources/json/stocksDB-Entities/export-incremental.json c2bc86757
>
>
> Diff: https://reviews.apache.org/r/68671/diff/2/
>
>
> Testing
> -------
>
> **Unit tests**
> Existing tests.
>
> **Volume tests**
> Export with 150 tables, each with 80 columns data.
>
> **Pre-commit**
> https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/657/
>
>
> Thanks,
>
> Ashutosh Mestry
>
>
Re: Review Request 68671: Export Service: Incremental Export:
Considers RequestContext's Timestamp
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68671/#review208489
-----------------------------------------------------------
Ship it!
Ship It!
- Madhan Neethiraj
On Sept. 10, 2018, 4:30 p.m., Ashutosh Mestry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68671/
> -----------------------------------------------------------
>
> (Updated Sept. 10, 2018, 4:30 p.m.)
>
>
> Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-2862
> https://issues.apache.org/jira/browse/ATLAS-2862
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Approach**
> Using _RequestContextV1.earliestActiveRequestTime_.
>
> Refactoring:
> - Removed _AtlasExportData.entities_ since this had entire payload of exported entities, causing memory bloat.
> - Changed _AtlasExport_ options' _fromTime_ to _changeToken_.
> - Additional refactoring for better readability.
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/model/impexp/AtlasExportRequest.java 96a6e8855
> intg/src/main/java/org/apache/atlas/model/impexp/AtlasExportResult.java fd6871219
> intg/src/main/java/org/apache/atlas/model/impexp/AtlasImportResult.java f066688ea
> repository/src/main/java/org/apache/atlas/repository/impexp/AuditsWriter.java 7fb2a3b61
> repository/src/main/java/org/apache/atlas/repository/impexp/ExportService.java b507002bd
> repository/src/main/java/org/apache/atlas/repository/impexp/ZipSink.java 17ebbf13e
> repository/src/test/java/org/apache/atlas/repository/impexp/ExportIncrementalTest.java ed4fc372a
> repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsShaperTest.java f89455318
> repository/src/test/java/org/apache/atlas/repository/impexp/ReplicationEntityAttributeTest.java 719d6ca09
> repository/src/test/java/org/apache/atlas/repository/impexp/ZipSinkTest.java e8bbeb590
> repository/src/test/resources/json/stocksDB-Entities/export-incremental.json c2bc86757
>
>
> Diff: https://reviews.apache.org/r/68671/diff/3/
>
>
> Testing
> -------
>
> **Unit tests**
> Existing tests.
>
> **Volume tests**
> Export with 150 tables, each with 80 columns data.
>
> **Pre-commit**
> https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/657/
>
>
> Thanks,
>
> Ashutosh Mestry
>
>
Re: Review Request 68671: Export Service: Incremental Export:
Considers RequestContext's Timestamp
Posted by Ashutosh Mestry <am...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68671/
-----------------------------------------------------------
(Updated Sept. 10, 2018, 4:30 p.m.)
Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
Changes
-------
Updates include: Addressed review comments.
Bugs: ATLAS-2862
https://issues.apache.org/jira/browse/ATLAS-2862
Repository: atlas
Description
-------
**Approach**
Using _RequestContextV1.earliestActiveRequestTime_.
Refactoring:
- Removed _AtlasExportData.entities_ since this had entire payload of exported entities, causing memory bloat.
- Changed _AtlasExport_ options' _fromTime_ to _changeToken_.
- Additional refactoring for better readability.
Diffs (updated)
-----
intg/src/main/java/org/apache/atlas/model/impexp/AtlasExportRequest.java 96a6e8855
intg/src/main/java/org/apache/atlas/model/impexp/AtlasExportResult.java fd6871219
intg/src/main/java/org/apache/atlas/model/impexp/AtlasImportResult.java f066688ea
repository/src/main/java/org/apache/atlas/repository/impexp/AuditsWriter.java 7fb2a3b61
repository/src/main/java/org/apache/atlas/repository/impexp/ExportService.java b507002bd
repository/src/main/java/org/apache/atlas/repository/impexp/ZipSink.java 17ebbf13e
repository/src/test/java/org/apache/atlas/repository/impexp/ExportIncrementalTest.java ed4fc372a
repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsShaperTest.java f89455318
repository/src/test/java/org/apache/atlas/repository/impexp/ReplicationEntityAttributeTest.java 719d6ca09
repository/src/test/java/org/apache/atlas/repository/impexp/ZipSinkTest.java e8bbeb590
repository/src/test/resources/json/stocksDB-Entities/export-incremental.json c2bc86757
Diff: https://reviews.apache.org/r/68671/diff/3/
Changes: https://reviews.apache.org/r/68671/diff/2-3/
Testing
-------
**Unit tests**
Existing tests.
**Volume tests**
Export with 150 tables, each with 80 columns data.
**Pre-commit**
https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/657/
Thanks,
Ashutosh Mestry
Re: Review Request 68671: Export Service: Incremental Export:
Considers RequestContext's Timestamp
Posted by Ashutosh Mestry <am...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68671/
-----------------------------------------------------------
(Updated Sept. 7, 2018, 10:14 p.m.)
Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
Changes
-------
Updates include: Addressed review comments.
Bugs: ATLAS-2862
https://issues.apache.org/jira/browse/ATLAS-2862
Repository: atlas
Description
-------
**Approach**
Using _RequestContextV1.earliestActiveRequestTime_.
Refactoring:
- Removed _AtlasExportData.entities_ since this had entire payload of exported entities, causing memory bloat.
- Changed _AtlasExport_ options' _fromTime_ to _changeToken_.
- Additional refactoring for better readability.
Diffs (updated)
-----
intg/src/main/java/org/apache/atlas/model/impexp/AtlasExportRequest.java 96a6e8855
intg/src/main/java/org/apache/atlas/model/impexp/AtlasExportResult.java fd6871219
intg/src/main/java/org/apache/atlas/model/impexp/AtlasImportResult.java f066688ea
repository/src/main/java/org/apache/atlas/repository/impexp/AuditsWriter.java 7fb2a3b61
repository/src/main/java/org/apache/atlas/repository/impexp/ExportService.java b507002bd
repository/src/main/java/org/apache/atlas/repository/impexp/ZipSink.java 17ebbf13e
repository/src/test/java/org/apache/atlas/repository/impexp/ExportIncrementalTest.java ed4fc372a
repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsShaperTest.java f89455318
repository/src/test/java/org/apache/atlas/repository/impexp/ReplicationEntityAttributeTest.java 719d6ca09
repository/src/test/java/org/apache/atlas/repository/impexp/ZipSinkTest.java e8bbeb590
repository/src/test/resources/json/stocksDB-Entities/export-incremental.json c2bc86757
Diff: https://reviews.apache.org/r/68671/diff/2/
Changes: https://reviews.apache.org/r/68671/diff/1-2/
Testing
-------
**Unit tests**
Existing tests.
**Volume tests**
Export with 150 tables, each with 80 columns data.
Thanks,
Ashutosh Mestry
Re: Review Request 68671: Export Service: Incremental Export:
Considers RequestContext's Timestamp
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68671/#review208462
-----------------------------------------------------------
intg/src/main/java/org/apache/atlas/model/impexp/AtlasExportRequest.java
Line 56 (original), 57 (patched)
<https://reviews.apache.org/r/68671/#comment292384>
CHANGE_TOKEN => MARKER
repository/src/main/java/org/apache/atlas/repository/impexp/ExportService.java
Lines 90 (patched)
<https://reviews.apache.org/r/68671/#comment292386>
RequestContextV1.earliestActiveRequestTime() ==> Math.min(RequestContextV1.earliestActiveRequestTime(), RequestContext.earliestActiveRequestTime())
repository/src/main/java/org/apache/atlas/repository/impexp/ExportService.java
Line 678 (original), 649 (patched)
<https://reviews.apache.org/r/68671/#comment292387>
<=
repository/src/main/java/org/apache/atlas/repository/impexp/ZipSink.java
Line 95 (original), 96 (patched)
<https://reviews.apache.org/r/68671/#comment292385>
if FILE_EXTENSION_JSON is assigned (& referenced only) inside "saveToZip()", it may not be necessary to use it as instance-member. Either assign its value in line #41 (and mark it final) or use a local variable.
- Madhan Neethiraj
On Sept. 7, 2018, 7 p.m., Ashutosh Mestry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68671/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2018, 7 p.m.)
>
>
> Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-2862
> https://issues.apache.org/jira/browse/ATLAS-2862
>
>
> Repository: atlas
>
>
> Description
> -------
>
> **Approach**
> Using _RequestContextV1.earliestActiveRequestTime_.
>
> Refactoring:
> - Removed _AtlasExportData.entities_ since this had entire payload of exported entities, causing memory bloat.
> - Changed _AtlasExport_ options' _fromTime_ to _changeToken_.
> - Additional refactoring for better readability.
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/model/impexp/AtlasExportRequest.java 96a6e8855
> intg/src/main/java/org/apache/atlas/model/impexp/AtlasExportResult.java fd6871219
> intg/src/main/java/org/apache/atlas/model/impexp/AtlasImportResult.java f066688ea
> repository/src/main/java/org/apache/atlas/repository/impexp/AuditsWriter.java 7fb2a3b61
> repository/src/main/java/org/apache/atlas/repository/impexp/ExportService.java b507002bd
> repository/src/main/java/org/apache/atlas/repository/impexp/ZipSink.java 17ebbf13e
> repository/src/test/java/org/apache/atlas/repository/impexp/ExportIncrementalTest.java ed4fc372a
> repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsShaperTest.java f89455318
> repository/src/test/java/org/apache/atlas/repository/impexp/ReplicationEntityAttributeTest.java 719d6ca09
> repository/src/test/java/org/apache/atlas/repository/impexp/ZipSinkTest.java e8bbeb590
> repository/src/test/resources/json/stocksDB-Entities/export-incremental.json c2bc86757
>
>
> Diff: https://reviews.apache.org/r/68671/diff/1/
>
>
> Testing
> -------
>
> **Unit tests**
> Existing tests.
>
> **Volume tests**
> Export with 150 tables, each with 80 columns data.
>
>
> Thanks,
>
> Ashutosh Mestry
>
>