You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Sarath Subramanian <sa...@apache.org> on 2021/03/04 06:55:10 UTC
Re: Review Request 73076: Deferred Actions Implementation

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


Fix it, then Ship it!





intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java
Lines 98 (patched)
<https://reviews.apache.org/r/73076/#comment311824>

    'pendingTasks'is never used?



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
Lines 2781 (patched)
<https://reviews.apache.org/r/73076/#comment311820>

    why createAndQueueTask() need both 'entityVertex' and 'entityGuid'. Consider passing only 'entityVertex' and retrieve 'entityGuid' from entityVertex to avoid duplicates in methods params.



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
Lines 2795 (patched)
<https://reviews.apache.org/r/73076/#comment311821>

    'entityGuid' and 'entityVertex' doesn't need to be duplicated in methods params. Fetch entityGuid from entityVertex.



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/tasks/ClassificationTask.java
Lines 78 (patched)
<https://reviews.apache.org/r/73076/#comment311823>

    remove task guid from entity vertex after task completion?
    
    entityGraphMapper.removePendingTaskFromEntityVertex(entityGuid, getTaskGuid())



repository/src/main/java/org/apache/atlas/tasks/TaskManagement.java
Lines 199 (patched)
<https://reviews.apache.org/r/73076/#comment311822>

    REPORT_FREQUENCEY => REPORT_FREQUENCY


- Sarath Subramanian


On Feb. 25, 2021, 10:08 p.m., Ashutosh Mestry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73076/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2021, 10:08 p.m.)
> 
> 
> Review request for atlas, Jayendra Parab, Madhan Neethiraj, Nikhil Bonte, Nixon Rodrigues, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3919
>     https://issues.apache.org/jira/browse/ATLAS-3919
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> **Approach**
> Model: _TaskDef_
> - Definition of task.
> - Contains status of the task for querying.
> 
> _AbstractTask_
> - Takes TaskDef as input and calls run.
> - The concrete tasks has logic for doing real work.
> 
> _TaskFactory_
> 
> - Creates tasks given task type.
> - Enumerates all the tasks that are supported.
> 
> _TaskManagement_
> 
> - Component/Service. Constructed after all existing components are constructed.
> - Provides a simple interface for adding tasks.
> - Processes tasks that are in pending state.
> - Supports HA mode, where the tasks are not executed on passive instance.
> 
> _TaskManagement.Registry_
> 
> - CRUD operations with TaskDef vertex.
> 
> _TaskManagement.Processor_
> - Provides mechanism for provides single queue for processing tasks in a way that is non-blocking.
> - Uses task type and the associated TaskFactory to construct concrete task.
> 
> Changes to _EntityGraphMapper_
> There are 3 new methods added that use portion of existing classification propagation logic and create a separate method.
> 
> New class _EntityGraphMapperWithTasks_ acts as a proxy for EntityGraphMapper. All calls that operate without tasks get channeled to the existing implementation. The ones involving tasks are modified to use the new task framework.
> 
> **Concurrent Changes to Classifications**
> Via REST APIs and operations from web UI, it is possible to make changes to the same entity. This situation will be more pronounced in the new setup.
> 
> The existing framework has _GraphTransactionInterceptor.lockObjectAndReleasePostCommit_. This provides ReentrantLock on an entity that is identified by the supplied GUID. This will cause a lock on the entity and all the impacted vertices. This has potential for adversely impacting scalability.
> 
> **REST APIs**
> +-----------------+--------+----------+----------------------------------------------------------------------+
> | api/admin/tasks   | GET  |  guids   |  (optional) List of guids to be fetched.                             |
> |                   |      |          | If list of guids supplied is empty all tasks                         |
> |                   |      |          | that are in PENDING state are fetched. If list                       |
> |                   |      |          | guids is specified, tasks corresponding to that guid are fetched.    |
> +-------------------+------+----------+----------------------------------------------------------------------+
> |api/admin/task     |DELETE| guids    | List of guids to be deleted. No action is taken if list of guids     |
> |                   |      |          | supplied is empty or a Task guid does not exist.                     |
> +-------------------+------+----------+----------------------------------------------------------------------+
> 
> **New Updates**
> - Separate log file for tasks.
> - TaskDef now has _startTime_ and _endTime_ to indicating start and end of task.
> - Refactored parameter passing during task creation. It now accepts Map instead of Object array. Each task creates its own set of parameters.
> - Reduced use of TASKS_ENABLE flag checks.
> - Added _AtlasEntity.getPendingTasks()_ that indicidate presence of tasks for a given entity.
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/atlas/repository/Constants.java 61abfcaca 
>   distro/src/conf/atlas-log4j.xml 7df963eb2 
>   intg/src/main/java/org/apache/atlas/AtlasConfiguration.java 08d6c9d4a 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 4d8c94894 
>   intg/src/main/java/org/apache/atlas/model/tasks/AtlasTask.java PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java 27c7f7391 
>   intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java 4c7f8c681 
>   intg/src/main/java/org/apache/atlas/type/Constants.java 7ee852086 
>   intg/src/test/resources/atlas-application.properties 7e74d5107 
>   repository/src/main/java/org/apache/atlas/GraphTransactionInterceptor.java 86b369fc9 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java d0ffb853f 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java 2cfcc0bc1 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java 8208d11c2 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v2/tasks/ClassificationPropagateTaskFactory.java PRE-CREATION 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v2/tasks/ClassificationPropagationTasks.java PRE-CREATION 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v2/tasks/ClassificationTask.java PRE-CREATION 
>   repository/src/main/java/org/apache/atlas/tasks/AbstractTask.java PRE-CREATION 
>   repository/src/main/java/org/apache/atlas/tasks/TaskExecutor.java PRE-CREATION 
>   repository/src/main/java/org/apache/atlas/tasks/TaskFactory.java PRE-CREATION 
>   repository/src/main/java/org/apache/atlas/tasks/TaskManagement.java PRE-CREATION 
>   repository/src/main/java/org/apache/atlas/tasks/TaskRegistry.java PRE-CREATION 
>   repository/src/test/java/org/apache/atlas/TestModules.java 71b0a4a68 
>   repository/src/test/java/org/apache/atlas/discovery/AtlasDiscoveryServiceTest.java 9846d4310 
>   repository/src/test/java/org/apache/atlas/repository/tagpropagation/ClassificationPropagationTestWithTasks.java PRE-CREATION 
>   repository/src/test/java/org/apache/atlas/tasks/TaskExecutorTest.java PRE-CREATION 
>   repository/src/test/java/org/apache/atlas/tasks/TaskManagementTest.java PRE-CREATION 
>   repository/src/test/java/org/apache/atlas/tasks/TaskRegistryTest.java PRE-CREATION 
>   server-api/src/main/java/org/apache/atlas/RequestContext.java 7de3536f4 
>   server-api/src/main/java/org/apache/atlas/listener/ActiveStateChangeHandler.java bb7f8fcc4 
>   test-tools/pom.xml ccad452ea 
>   test-tools/src/main/resources/log4j.properties 4db0598ad 
>   test-tools/src/main/resources/solr/core-template/solrconfig.xml 4d7e444bf 
>   webapp/src/main/java/org/apache/atlas/BeanUtil.java ef5a741ee 
>   webapp/src/main/java/org/apache/atlas/web/resources/AdminResource.java e8fc111a6 
>   webapp/src/main/java/org/apache/atlas/web/service/EmbeddedServer.java d6658fe29 
>   webapp/src/test/java/org/apache/atlas/web/resources/AdminResourceTest.java 77422b2a5 
> 
> 
> Diff: https://reviews.apache.org/r/73076/diff/20/
> 
> 
> Testing
> -------
> 
> **Unit tests**
> New tests.
> Additional unit tests covering end to end scenarios.
> 
> **Manual tests**
> - Via web ui.
> - REST APIs.
> 
> **Volume test**
> - Adding classification with propagations for lineage with 2000 elements.
> 
> **Pre-commit build**
> https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/281/
> 
> 
> Thanks,
> 
> Ashutosh Mestry
> 
>