You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Miklos Gergely <mg...@hortonworks.com> on 2019/04/23 13:23:36 UTC
Review Request 70524: Break up DDLTask - extract Workload Management
related operations
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70524/
-----------------------------------------------------------
Review request for hive and Zoltan Haindrich.
Bugs: HIVE-21635
https://issues.apache.org/jira/browse/HIVE-21635
Repository: hive-git
Description
-------
DDLTask is a huge class, more than 5000 lines long. The related DDLWork is also a huge class, which has a field for each DDL operation it supports. The goal is to refactor these in order to have everything cut into more handleable classes under the package org.apache.hadoop.hive.ql.exec.ddl:
have a separate class for each operation
have a package for each operation group (database ddl, table ddl, etc), so the amount of classes under a package is more manageable
make all the requests (DDLDesc subclasses) immutable
DDLTask should be agnostic to the actual operations
right now let's ignore the issue of having some operations handled by DDLTask which are not actual DDL operations (lock, unlock, desc...)
In the interim time when there are two DDLTask and DDLWork classes in the code base the new ones in the new package are called DDLTask2 and DDLWork2 thus avoiding the usage of fully qualified class names where both the old and the new classes are in use.
Step #6: extract all the workload management related operations from the old DDLTask, and move them under the new package.
Diffs
-----
ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLWork2.java a2f49b7503
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/CreateRoleOperation.java 6782b02d20
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/DropRoleOperation.java e8b55ecf4c
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/GrantOperation.java 633ac434e0
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/GrantRoleOperation.java 19abe2794d
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/RevokeOperation.java bf4e01a191
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/RevokeRoleOperation.java 0b3b27695d
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/RoleUtils.java cfbc4cf620
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/SetRoleOperation.java d119fe4a28
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowCurrentRoleOperation.java 9738ddbcc0
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowGrantOperation.java 50b41800a1
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowPrincipalsOperation.java 392142ba14
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowRoleGrantOperation.java 178ea8e3bc
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowRolesOperation.java 22ca7f350d
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterPoolAddTriggerDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterPoolAddTriggerOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterPoolDropTriggerDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterPoolDropTriggerOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterResourcePlanOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMMappingOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMPoolDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMPoolOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMTriggerOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateResourcePlanOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMMappingDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMMappingOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMPoolDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMPoolOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMTriggerOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/DropResourcePlanOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/DropWMMappingOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/DropWMPoolOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/DropWMTriggerOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/ShowResourcePlanOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/WMUtils.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/package-info.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 13d7d6fcdb
ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java d2c3f7b1a6
ql/src/java/org/apache/hadoop/hive/ql/plan/AlterResourcePlanDesc.java dd2eaced23
ql/src/java/org/apache/hadoop/hive/ql/plan/AlterWMTriggerDesc.java 677a47caca
ql/src/java/org/apache/hadoop/hive/ql/plan/CreateOrAlterWMMappingDesc.java 148e73212c
ql/src/java/org/apache/hadoop/hive/ql/plan/CreateOrAlterWMPoolDesc.java 53f1f71399
ql/src/java/org/apache/hadoop/hive/ql/plan/CreateResourcePlanDesc.java 9c18f59d09
ql/src/java/org/apache/hadoop/hive/ql/plan/CreateWMTriggerDesc.java 8eb97291ff
ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java e6f3a6f917
ql/src/java/org/apache/hadoop/hive/ql/plan/DropResourcePlanDesc.java efaf0789b0
ql/src/java/org/apache/hadoop/hive/ql/plan/DropWMMappingDesc.java 54becd6283
ql/src/java/org/apache/hadoop/hive/ql/plan/DropWMPoolDesc.java cad6c1e396
ql/src/java/org/apache/hadoop/hive/ql/plan/DropWMTriggerDesc.java da7c18310c
ql/src/java/org/apache/hadoop/hive/ql/plan/ShowResourcePlanDesc.java 370f0d9e78
ql/src/test/queries/clientpositive/resourceplan.q 93d848ba33
ql/src/test/results/clientpositive/llap/resourceplan.q.out c0d6ec2905
ql/src/test/results/clientpositive/resourceplan.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/70524/diff/1/
Testing
-------
All the unit tests are still running + added some new lines to cover explains.
Thanks,
Miklos Gergely
Re: Review Request 70524: Break up DDLTask - extract Workload
Management related operations
Posted by Miklos Gergely <mg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70524/
-----------------------------------------------------------
(Updated April 23, 2019, 4:59 p.m.)
Review request for hive and Zoltan Haindrich.
Changes
-------
Fixed comments wm -> workload management
Bugs: HIVE-21635
https://issues.apache.org/jira/browse/HIVE-21635
Repository: hive-git
Description
-------
DDLTask is a huge class, more than 5000 lines long. The related DDLWork is also a huge class, which has a field for each DDL operation it supports. The goal is to refactor these in order to have everything cut into more handleable classes under the package org.apache.hadoop.hive.ql.exec.ddl:
have a separate class for each operation
have a package for each operation group (database ddl, table ddl, etc), so the amount of classes under a package is more manageable
make all the requests (DDLDesc subclasses) immutable
DDLTask should be agnostic to the actual operations
right now let's ignore the issue of having some operations handled by DDLTask which are not actual DDL operations (lock, unlock, desc...)
In the interim time when there are two DDLTask and DDLWork classes in the code base the new ones in the new package are called DDLTask2 and DDLWork2 thus avoiding the usage of fully qualified class names where both the old and the new classes are in use.
Step #6: extract all the workload management related operations from the old DDLTask, and move them under the new package.
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLWork2.java a2f49b7503
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/CreateRoleOperation.java 6782b02d20
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/DropRoleOperation.java e8b55ecf4c
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/GrantOperation.java 633ac434e0
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/GrantRoleOperation.java 19abe2794d
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/RevokeOperation.java bf4e01a191
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/RevokeRoleOperation.java 0b3b27695d
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/RoleUtils.java cfbc4cf620
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/SetRoleOperation.java d119fe4a28
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowCurrentRoleOperation.java 9738ddbcc0
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowGrantOperation.java 50b41800a1
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowPrincipalsOperation.java 392142ba14
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowRoleGrantOperation.java 178ea8e3bc
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowRolesOperation.java 22ca7f350d
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterPoolAddTriggerDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterPoolAddTriggerOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterPoolDropTriggerDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterPoolDropTriggerOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterResourcePlanOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMMappingOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMPoolDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMPoolOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMTriggerOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateResourcePlanOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMMappingDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMMappingOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMPoolDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMPoolOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMTriggerOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/DropResourcePlanOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/DropWMMappingOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/DropWMPoolOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/DropWMTriggerOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/ShowResourcePlanOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/WMUtils.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/package-info.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 13d7d6fcdb
ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java d2c3f7b1a6
ql/src/java/org/apache/hadoop/hive/ql/plan/AlterResourcePlanDesc.java dd2eaced23
ql/src/java/org/apache/hadoop/hive/ql/plan/AlterWMTriggerDesc.java 677a47caca
ql/src/java/org/apache/hadoop/hive/ql/plan/CreateOrAlterWMMappingDesc.java 148e73212c
ql/src/java/org/apache/hadoop/hive/ql/plan/CreateOrAlterWMPoolDesc.java 53f1f71399
ql/src/java/org/apache/hadoop/hive/ql/plan/CreateResourcePlanDesc.java 9c18f59d09
ql/src/java/org/apache/hadoop/hive/ql/plan/CreateWMTriggerDesc.java 8eb97291ff
ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java e6f3a6f917
ql/src/java/org/apache/hadoop/hive/ql/plan/DropResourcePlanDesc.java efaf0789b0
ql/src/java/org/apache/hadoop/hive/ql/plan/DropWMMappingDesc.java 54becd6283
ql/src/java/org/apache/hadoop/hive/ql/plan/DropWMPoolDesc.java cad6c1e396
ql/src/java/org/apache/hadoop/hive/ql/plan/DropWMTriggerDesc.java da7c18310c
ql/src/java/org/apache/hadoop/hive/ql/plan/ShowResourcePlanDesc.java 370f0d9e78
ql/src/test/queries/clientpositive/resourceplan.q 93d848ba33
ql/src/test/results/clientpositive/llap/resourceplan.q.out c0d6ec2905
ql/src/test/results/clientpositive/resourceplan.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/70524/diff/2/
Changes: https://reviews.apache.org/r/70524/diff/1-2/
Testing
-------
All the unit tests are still running + added some new lines to cover explains.
Thanks,
Miklos Gergely
Re: Review Request 70524: Break up DDLTask - extract Workload
Management related operations
Posted by Miklos Gergely <mg...@hortonworks.com>.
> On April 23, 2019, 3:32 p.m., Andrew Sherman wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMMappingOperation.java
> > Lines 28 (patched)
> > <https://reviews.apache.org/r/70524/diff/1/?file=2140708#file2140708line28>
> >
> > This is a nit, but if you spell out "workload management" rather than "wm" in the javadoc then the classes will be easier for the naive reader to understand.
Fixed, thanks
- Miklos
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70524/#review214816
-----------------------------------------------------------
On April 23, 2019, 4:59 p.m., Miklos Gergely wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70524/
> -----------------------------------------------------------
>
> (Updated April 23, 2019, 4:59 p.m.)
>
>
> Review request for hive and Zoltan Haindrich.
>
>
> Bugs: HIVE-21635
> https://issues.apache.org/jira/browse/HIVE-21635
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> DDLTask is a huge class, more than 5000 lines long. The related DDLWork is also a huge class, which has a field for each DDL operation it supports. The goal is to refactor these in order to have everything cut into more handleable classes under the package org.apache.hadoop.hive.ql.exec.ddl:
>
> have a separate class for each operation
> have a package for each operation group (database ddl, table ddl, etc), so the amount of classes under a package is more manageable
> make all the requests (DDLDesc subclasses) immutable
> DDLTask should be agnostic to the actual operations
> right now let's ignore the issue of having some operations handled by DDLTask which are not actual DDL operations (lock, unlock, desc...)
> In the interim time when there are two DDLTask and DDLWork classes in the code base the new ones in the new package are called DDLTask2 and DDLWork2 thus avoiding the usage of fully qualified class names where both the old and the new classes are in use.
>
> Step #6: extract all the workload management related operations from the old DDLTask, and move them under the new package.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLWork2.java a2f49b7503
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/CreateRoleOperation.java 6782b02d20
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/DropRoleOperation.java e8b55ecf4c
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/GrantOperation.java 633ac434e0
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/GrantRoleOperation.java 19abe2794d
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/RevokeOperation.java bf4e01a191
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/RevokeRoleOperation.java 0b3b27695d
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/RoleUtils.java cfbc4cf620
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/SetRoleOperation.java d119fe4a28
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowCurrentRoleOperation.java 9738ddbcc0
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowGrantOperation.java 50b41800a1
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowPrincipalsOperation.java 392142ba14
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowRoleGrantOperation.java 178ea8e3bc
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowRolesOperation.java 22ca7f350d
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterPoolAddTriggerDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterPoolAddTriggerOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterPoolDropTriggerDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterPoolDropTriggerOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterResourcePlanOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMMappingOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMPoolDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMPoolOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMTriggerOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateResourcePlanOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMMappingDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMMappingOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMPoolDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMPoolOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMTriggerOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/DropResourcePlanOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/DropWMMappingOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/DropWMPoolOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/DropWMTriggerOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/ShowResourcePlanOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/WMUtils.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/package-info.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 13d7d6fcdb
> ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java d2c3f7b1a6
> ql/src/java/org/apache/hadoop/hive/ql/plan/AlterResourcePlanDesc.java dd2eaced23
> ql/src/java/org/apache/hadoop/hive/ql/plan/AlterWMTriggerDesc.java 677a47caca
> ql/src/java/org/apache/hadoop/hive/ql/plan/CreateOrAlterWMMappingDesc.java 148e73212c
> ql/src/java/org/apache/hadoop/hive/ql/plan/CreateOrAlterWMPoolDesc.java 53f1f71399
> ql/src/java/org/apache/hadoop/hive/ql/plan/CreateResourcePlanDesc.java 9c18f59d09
> ql/src/java/org/apache/hadoop/hive/ql/plan/CreateWMTriggerDesc.java 8eb97291ff
> ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java e6f3a6f917
> ql/src/java/org/apache/hadoop/hive/ql/plan/DropResourcePlanDesc.java efaf0789b0
> ql/src/java/org/apache/hadoop/hive/ql/plan/DropWMMappingDesc.java 54becd6283
> ql/src/java/org/apache/hadoop/hive/ql/plan/DropWMPoolDesc.java cad6c1e396
> ql/src/java/org/apache/hadoop/hive/ql/plan/DropWMTriggerDesc.java da7c18310c
> ql/src/java/org/apache/hadoop/hive/ql/plan/ShowResourcePlanDesc.java 370f0d9e78
> ql/src/test/queries/clientpositive/resourceplan.q 93d848ba33
> ql/src/test/results/clientpositive/llap/resourceplan.q.out c0d6ec2905
> ql/src/test/results/clientpositive/resourceplan.q.out PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/70524/diff/2/
>
>
> Testing
> -------
>
> All the unit tests are still running + added some new lines to cover explains.
>
>
> Thanks,
>
> Miklos Gergely
>
>
Re: Review Request 70524: Break up DDLTask - extract Workload
Management related operations
Posted by Andrew Sherman via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70524/#review214816
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMMappingOperation.java
Lines 28 (patched)
<https://reviews.apache.org/r/70524/#comment301065>
This is a nit, but if you spell out "workload management" rather than "wm" in the javadoc then the classes will be easier for the naive reader to understand.
- Andrew Sherman
On April 23, 2019, 1:23 p.m., Miklos Gergely wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70524/
> -----------------------------------------------------------
>
> (Updated April 23, 2019, 1:23 p.m.)
>
>
> Review request for hive and Zoltan Haindrich.
>
>
> Bugs: HIVE-21635
> https://issues.apache.org/jira/browse/HIVE-21635
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> DDLTask is a huge class, more than 5000 lines long. The related DDLWork is also a huge class, which has a field for each DDL operation it supports. The goal is to refactor these in order to have everything cut into more handleable classes under the package org.apache.hadoop.hive.ql.exec.ddl:
>
> have a separate class for each operation
> have a package for each operation group (database ddl, table ddl, etc), so the amount of classes under a package is more manageable
> make all the requests (DDLDesc subclasses) immutable
> DDLTask should be agnostic to the actual operations
> right now let's ignore the issue of having some operations handled by DDLTask which are not actual DDL operations (lock, unlock, desc...)
> In the interim time when there are two DDLTask and DDLWork classes in the code base the new ones in the new package are called DDLTask2 and DDLWork2 thus avoiding the usage of fully qualified class names where both the old and the new classes are in use.
>
> Step #6: extract all the workload management related operations from the old DDLTask, and move them under the new package.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLWork2.java a2f49b7503
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/CreateRoleOperation.java 6782b02d20
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/DropRoleOperation.java e8b55ecf4c
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/GrantOperation.java 633ac434e0
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/GrantRoleOperation.java 19abe2794d
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/RevokeOperation.java bf4e01a191
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/RevokeRoleOperation.java 0b3b27695d
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/RoleUtils.java cfbc4cf620
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/SetRoleOperation.java d119fe4a28
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowCurrentRoleOperation.java 9738ddbcc0
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowGrantOperation.java 50b41800a1
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowPrincipalsOperation.java 392142ba14
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowRoleGrantOperation.java 178ea8e3bc
> ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowRolesOperation.java 22ca7f350d
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterPoolAddTriggerDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterPoolAddTriggerOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterPoolDropTriggerDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterPoolDropTriggerOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterResourcePlanOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMMappingOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMPoolDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMPoolOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/AlterWMTriggerOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateResourcePlanOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMMappingDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMMappingOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMPoolDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMPoolOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/CreateWMTriggerOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/DropResourcePlanOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/DropWMMappingOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/DropWMPoolOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/DropWMTriggerOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/ShowResourcePlanOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/WMUtils.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/workloadmanagement/package-info.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 13d7d6fcdb
> ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java d2c3f7b1a6
> ql/src/java/org/apache/hadoop/hive/ql/plan/AlterResourcePlanDesc.java dd2eaced23
> ql/src/java/org/apache/hadoop/hive/ql/plan/AlterWMTriggerDesc.java 677a47caca
> ql/src/java/org/apache/hadoop/hive/ql/plan/CreateOrAlterWMMappingDesc.java 148e73212c
> ql/src/java/org/apache/hadoop/hive/ql/plan/CreateOrAlterWMPoolDesc.java 53f1f71399
> ql/src/java/org/apache/hadoop/hive/ql/plan/CreateResourcePlanDesc.java 9c18f59d09
> ql/src/java/org/apache/hadoop/hive/ql/plan/CreateWMTriggerDesc.java 8eb97291ff
> ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java e6f3a6f917
> ql/src/java/org/apache/hadoop/hive/ql/plan/DropResourcePlanDesc.java efaf0789b0
> ql/src/java/org/apache/hadoop/hive/ql/plan/DropWMMappingDesc.java 54becd6283
> ql/src/java/org/apache/hadoop/hive/ql/plan/DropWMPoolDesc.java cad6c1e396
> ql/src/java/org/apache/hadoop/hive/ql/plan/DropWMTriggerDesc.java da7c18310c
> ql/src/java/org/apache/hadoop/hive/ql/plan/ShowResourcePlanDesc.java 370f0d9e78
> ql/src/test/queries/clientpositive/resourceplan.q 93d848ba33
> ql/src/test/results/clientpositive/llap/resourceplan.q.out c0d6ec2905
> ql/src/test/results/clientpositive/resourceplan.q.out PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/70524/diff/1/
>
>
> Testing
> -------
>
> All the unit tests are still running + added some new lines to cover explains.
>
>
> Thanks,
>
> Miklos Gergely
>
>