sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jarek Cecho" <jar...@apache.org>
Subject Re: Review Request 27330: SQOOP-1510:JobRequestHandler for submit/abort job and SubmissionHandler changes
Date Fri, 31 Oct 2014 13:33:19 GMT


> On Oct. 30, 2014, 1:28 a.m., Abraham Elmahrek wrote:
> > I think this patch blew up way out of scope. There are changes to the CLI that don't
appear to be necessary immediately or can be moved to separate Jiras. For example, the "start"
command is gone and replaced by a "submit" command. All of these changes have actually broken
pieces of the shell client. Try running the following commands from the shell client:
> > # show job --all
> > # show connector --all
> > # help
> > 
> > Think it may be worth while to address these pieces separately?
> 
> Veena Basavaraj wrote:
>     1. All the changes here are in the scope of the ticket and the changes were discsused
before. It is in the PDF. 
>     2. I cannot afford to break the CLI and make just the rest api changes. This has
been the same principle I have used so far for the 4 other tickets in 1509, anytime I make
the rest api change if I dont do CLI change integration tests will break
>     3. I tried to test the commands related to JOB, all of this is manual which sucks,
but I awill look into it soon and see why the connector is broken. will upload a patch there.
> 
> Veena Basavaraj wrote:
>     i tested the latest again and both commands work. Not sure what you were testing,
let me know if you want me to send the sqoop war across

We had a "submission" command in the past and we got feedback that it's confusing, so we've
changed it to "start". Hence changing it back do yields need to have a separate JIRA with
proper discussion, so that we have track why we are going back.


> On Oct. 30, 2014, 1:28 a.m., Abraham Elmahrek wrote:
> > core/src/main/java/org/apache/sqoop/driver/JobManager.java, line 502
> > <https://reviews.apache.org/r/27330/diff/4/?file=742083#file742083line502>
> >
> >     This method just destroys the submission? Or does it test for job failure as
well?
> 
> Veena Basavaraj wrote:
>     it is called only on job failure which is surprising to me, good you noticed it can
you confirm it is expected to be called only on failure.
> 
> Veena Basavaraj wrote:
>     boolean success = submissionEngine.submit(jobRequest);
>           if (!success) {
>             destroySubmissionOnJobSubmitFailure(jobRequest);
>             mJobSubmission.setStatus(SubmissionStatus.FAILURE_ON_SUBMIT);
>           }

Yup, this method is called only on submit failure, because in normal case "destroyers" are
called from execution engine. As the submit has failed, the execution engine is not responsible
for calling destroyers and hence we have to call them explicitly - it's sort of a corner case.
In case of mapreduce, we're properly calling the destroyers in outputcommitter.


- Jarek


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


On Oct. 30, 2014, 9:08 p.m., Veena Basavaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27330/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2014, 9:08 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> see JIRA and its parent JIRA for details
> 
> All the WS stuff, I will do it in the end once the functionality is reviewd.
> 
> 
> If someone is wondering why START is changed to submit: there are tons of places in the
code and in the java docs, we actually mean submit when we say START
> 
>   DRIVER_0008("Invalid combination of submission and execution engines"),
> 
>   DRIVER_0009("Job has been disabled. Cannot submit this job."),
> 
>   DRIVER_0010("Link for this job has been disabled. Cannot submit this job."),
> 
>   DRIVER_0011("Connector does not support specified direction. Cannot submit this job."),
> 
>   ;
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/sqoop/client/SqoopClient.java 33a0c3c 
>   client/src/main/java/org/apache/sqoop/client/SubmissionCallback.java de7211a 
>   client/src/main/java/org/apache/sqoop/client/request/JobResourceRequest.java 83c08b3

>   client/src/main/java/org/apache/sqoop/client/request/SqoopResourceRequests.java 4a56bb7

>   client/src/main/java/org/apache/sqoop/client/request/SubmissionResourceRequest.java
5055783 
>   common/src/main/java/org/apache/sqoop/json/JobBean.java 082d591 
>   common/src/main/java/org/apache/sqoop/json/JobsBean.java PRE-CREATION 
>   common/src/main/java/org/apache/sqoop/json/JsonBean.java ba86511 
>   common/src/main/java/org/apache/sqoop/json/LinksBean.java 5858a18 
>   common/src/main/java/org/apache/sqoop/json/SubmissionBean.java 4b80338 
>   common/src/main/java/org/apache/sqoop/json/SubmissionsBean.java PRE-CREATION 
>   common/src/main/java/org/apache/sqoop/model/MLink.java 7a9f538 
>   common/src/main/java/org/apache/sqoop/model/MSubmission.java 7290df5 
>   common/src/test/java/org/apache/sqoop/json/TestJobBean.java 1fc8dbd 
>   common/src/test/java/org/apache/sqoop/json/TestLinkBean.java 811cbf0 
>   common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java e4d50bf 
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcPartitioner.java
2411169 
>   connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestPartitioner.java
3ae64f0 
>   core/src/main/java/org/apache/sqoop/driver/JobManager.java f6447c6 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 2aeb07e 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ad380d3 
>   core/src/main/java/org/apache/sqoop/repository/Repository.java 79742b9 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
b996a0b 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java
c894d06 
>   repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java
4c2d062 
>   server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java 5694ea5

>   server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java 5547988 
>   server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java 35a9635 
>   server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java 8555b0c

>   server/src/main/java/org/apache/sqoop/server/RequestHandler.java 508edd2 
>   server/src/main/java/org/apache/sqoop/server/v1/JobServlet.java d295237 
>   server/src/main/java/org/apache/sqoop/server/v1/JobsServlet.java PRE-CREATION 
>   server/src/main/java/org/apache/sqoop/server/v1/LinkServlet.java 127903a 
>   server/src/main/java/org/apache/sqoop/server/v1/SubmissionServlet.java 5c1d883 
>   server/src/main/java/org/apache/sqoop/server/v1/SubmissionsServlet.java PRE-CREATION

>   server/src/main/webapp/WEB-INF/web.xml 6ad90d2 
>   shell/src/main/java/org/apache/sqoop/shell/AbortCommand.java PRE-CREATION 
>   shell/src/main/java/org/apache/sqoop/shell/ShowJobStatusFunction.java PRE-CREATION

>   shell/src/main/java/org/apache/sqoop/shell/SqoopShell.java 2e87965 
>   shell/src/main/java/org/apache/sqoop/shell/StartCommand.java 7c56980 
>   shell/src/main/java/org/apache/sqoop/shell/StartJobFunction.java 4363f05 
>   shell/src/main/java/org/apache/sqoop/shell/StatusCommand.java 3447a87 
>   shell/src/main/java/org/apache/sqoop/shell/StatusJobFunction.java fb83af3 
>   shell/src/main/java/org/apache/sqoop/shell/StopCommand.java 50b2e81 
>   shell/src/main/java/org/apache/sqoop/shell/StopJobFunction.java 790c522 
>   shell/src/main/java/org/apache/sqoop/shell/SubmitCommand.java PRE-CREATION 
>   shell/src/main/java/org/apache/sqoop/shell/SubmitJobFunction.java PRE-CREATION 
>   shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java dd075d7 
>   shell/src/main/java/org/apache/sqoop/shell/UpdateLinkFunction.java 176833a 
>   shell/src/main/java/org/apache/sqoop/shell/core/Constants.java 44d5920 
>   shell/src/main/resources/shell-resource.properties 0e63c50 
>   test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java af31769 
>   test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromRDBMSToHDFSTest.java
36f7443 
>   tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryDumpTool.java c219e68 
>   tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryLoadTool.java 64b08fc 
> 
> Diff: https://reviews.apache.org/r/27330/diff/
> 
> 
> Testing
> -------
> 
> yes
> 
> 
> Thanks,
> 
> Veena Basavaraj
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message