You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pig.apache.org by "Benjamin Reed (JIRA)" <ji...@apache.org> on 2009/02/17 20:14:59 UTC
[jira] Created: (PIG-682) Fix the ssh tunneling code
Fix the ssh tunneling code
--------------------------
Key: PIG-682
URL: https://issues.apache.org/jira/browse/PIG-682
Project: Pig
Issue Type: Bug
Components: impl
Reporter: Benjamin Reed
Hadoop has changed a bit and the ssh-gateway code no longer works. pig needs to be updated to register with the new socket framework. reporting of problems also needs to be better.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (PIG-682) Fix the ssh tunneling code
Posted by "Santhosh Srinivasan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-682?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12674378#action_12674378 ]
sms edited comment on PIG-682 at 2/17/09 2:37 PM:
------------------------------------------------------------------
Review comments:
General note:
1. The list of error codes and associated strings are documented on the wiki at:
http://wiki.apache.org/pig/PigErrorHandlingFunctionalSpecification
Look for the section titled "Compendium of error messages"
src/org/apache/pig/Main.java
=====================
1. The following code is not required. ExecException is now subclassed from PigException.
{code}
+ } catch (ExecException e) {
+ System.err.println("Exec error: " + e.getMessage());
+ rc = 2;
{code}
2. We should not be printing stack traces anymore. Exceptions that occur in main (within the execution part are already handled by Grunt.java. I could not think of use cases where there was an unhandled exception within Grunt and we did not report it as such.
{code}
+ } catch (Throwable e) {
rc = 2;
+ System.err.println("Unrecoverable error: " + e.getMessage());
+ e.printStackTrace();
{code}
Index: lib-src/shock/org/apache/pig/shock/SSHSocketImplFactory.java
===================================================================
1. If its a non-fatal error, log.warn is appropriate and not log.error
{code}
public SocketImpl createSocketImpl() {
- return new SSHSocketImpl(session);
-
+ try {
+ return new SSHSocketImpl(session);
+ } catch(Throwable e) {
+ log.error("Couldn't create impl", e);
+ return null;
+ }
}
{code}
Index: src/org/apache/pig/backend/hadoop/executionengine/HExecutionEngine.java
===================================================================
The setSSHFactory() method catches the SocketException but does nothing. Is this behaviour expected?
{code}
catch (SocketException e) {}
{code}
was (Author: sms):
Review comments:
General note:
1. The list of error codes and associated strings are documented on the wiki at:
http://wiki.apache.org/pig/PigErrorHandlingFunctionalSpecification
Look for the section titled "Compendium of error messages"
src/org/apache/pig/Main.java
=====================
1. The following code is not required. ExecException is now subclassed from PigException.
+ } catch (ExecException e) {
+ System.err.println("Exec error: " + e.getMessage());
+ rc = 2;
2. We should not be printing stack traces anymore. Exceptions that occur in main (within the execution part are already handled by Grunt.java. I could not think of use cases where there was an unhandled exception within Grunt and we did not report it as such.
+ } catch (Throwable e) {
rc = 2;
+ System.err.println("Unrecoverable error: " + e.getMessage());
+ e.printStackTrace();
Index: lib-src/shock/org/apache/pig/shock/SSHSocketImplFactory.java
===================================================================
1. If its a non-fatal error, log.warn is appropriate and not log.error
public SocketImpl createSocketImpl() {
- return new SSHSocketImpl(session);
-
+ try {
+ return new SSHSocketImpl(session);
+ } catch(Throwable e) {
+ log.error("Couldn't create impl", e);
+ return null;
+ }
}
Index: src/org/apache/pig/backend/hadoop/executionengine/HExecutionEngine.java
===================================================================
The setSSHFactory() method catches the SocketException but does nothing. Is this behaviour expected?
catch (SocketException e) {}
> Fix the ssh tunneling code
> --------------------------
>
> Key: PIG-682
> URL: https://issues.apache.org/jira/browse/PIG-682
> Project: Pig
> Issue Type: Bug
> Components: impl
> Reporter: Benjamin Reed
> Attachments: jsch-0.1.41.jar, PIG-682.patch
>
>
> Hadoop has changed a bit and the ssh-gateway code no longer works. pig needs to be updated to register with the new socket framework. reporting of problems also needs to be better.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (PIG-682) Fix the ssh tunneling code
Posted by "Olga Natkovich (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-682?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12674365#action_12674365 ]
Olga Natkovich commented on PIG-682:
------------------------------------
The patch looks fine but the error handling does not look quite consistent with new error handling code. I asked Santhosh to review that part.
> Fix the ssh tunneling code
> --------------------------
>
> Key: PIG-682
> URL: https://issues.apache.org/jira/browse/PIG-682
> Project: Pig
> Issue Type: Bug
> Components: impl
> Reporter: Benjamin Reed
> Attachments: jsch-0.1.41.jar, PIG-682.patch
>
>
> Hadoop has changed a bit and the ssh-gateway code no longer works. pig needs to be updated to register with the new socket framework. reporting of problems also needs to be better.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (PIG-682) Fix the ssh tunneling code
Posted by "Benjamin Reed (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-682?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Benjamin Reed updated PIG-682:
------------------------------
Attachment: jsch-0.1.41.jar
This version of sch seems to be robust enough to allow large file (multi-block) transfers over the ssh tunnel.
> Fix the ssh tunneling code
> --------------------------
>
> Key: PIG-682
> URL: https://issues.apache.org/jira/browse/PIG-682
> Project: Pig
> Issue Type: Bug
> Components: impl
> Reporter: Benjamin Reed
> Attachments: jsch-0.1.41.jar, PIG-682.patch
>
>
> Hadoop has changed a bit and the ssh-gateway code no longer works. pig needs to be updated to register with the new socket framework. reporting of problems also needs to be better.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (PIG-682) Fix the ssh tunneling code
Posted by "Benjamin Reed (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-682?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Benjamin Reed updated PIG-682:
------------------------------
Status: Patch Available (was: Open)
> Fix the ssh tunneling code
> --------------------------
>
> Key: PIG-682
> URL: https://issues.apache.org/jira/browse/PIG-682
> Project: Pig
> Issue Type: Bug
> Components: impl
> Reporter: Benjamin Reed
> Attachments: jsch-0.1.41.jar, PIG-682.patch
>
>
> Hadoop has changed a bit and the ssh-gateway code no longer works. pig needs to be updated to register with the new socket framework. reporting of problems also needs to be better.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (PIG-682) Fix the ssh tunneling code
Posted by "Benjamin Reed (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-682?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Benjamin Reed updated PIG-682:
------------------------------
Attachment: PIG-682.patch
This patch updates the ssh tunneling code to work with the new hadoop socket framework. it also enhances problem reporting so that things like client/server protocol mismatch and hod errors are more visible.
> Fix the ssh tunneling code
> --------------------------
>
> Key: PIG-682
> URL: https://issues.apache.org/jira/browse/PIG-682
> Project: Pig
> Issue Type: Bug
> Components: impl
> Reporter: Benjamin Reed
> Attachments: jsch-0.1.41.jar, PIG-682.patch
>
>
> Hadoop has changed a bit and the ssh-gateway code no longer works. pig needs to be updated to register with the new socket framework. reporting of problems also needs to be better.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (PIG-682) Fix the ssh tunneling code
Posted by "Pradeep Kamath (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-682?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12676810#action_12676810 ]
Pradeep Kamath commented on PIG-682:
------------------------------------
As noted in https://issues.apache.org/jira/browse/PIG-591?focusedCommentId=12676808#action_12676808 a part of this patch has already been committed as part of https://issues.apache.org/jira/browse/PIG-591. The portion which has already been committed is in HExecutionEngine.java:
{code}
@@ -200,7 +200,7 @@
}
catch (IOException e) {
int errCode = 6009;
- String msg = "Failed to create job client";
+ String msg = "Failed to create job client:" + e.getMessage();
throw new ExecException(msg, errCode, PigException.BUG, e);
}
}
@@ -549,11 +549,20 @@
//this should return as soon as connection is shutdown
int rc = p.waitFor();
if (rc != 0) {
- String errMsg = new String();
+ StringBuilder errMsg = new StringBuilder();
try {
- BufferedReader br = new BufferedReader(new InputStreamReader(p.getErrorStream()));
- errMsg = br.readLine();
+ BufferedReader br = new BufferedReader(new InputStreamReader(p.getInputStream()));
+ String line = null;
+ while((line = br.readLine()) != null) {
+ errMsg.append(line);
+ }
br.close();
+ br = new BufferedReader(new InputStreamReader(p.getErrorStream()));
+ line = null;
+ while((line = br.readLine()) != null) {
+ errMsg.append(line);
+ }
+ br.close();
} catch (IOException ioe) {}
int errCode = 6011;
StringBuilder msg = new StringBuilder("Failed to run command ");
@@ -563,7 +572,7 @@
msg.append("; return code: ");
msg.append(rc);
msg.append("; error: ");
- msg.append(errMsg);
+ msg.append(errMsg.toString());
throw new ExecException(msg.toString(), errCode, PigException.REMOTE_ENVIRONMENT);
}
} catch (Exception e){
{code}
When a new revision of this patch is generated to make the changes for the previous review comment, the above portion of code changes can be omitted.
> Fix the ssh tunneling code
> --------------------------
>
> Key: PIG-682
> URL: https://issues.apache.org/jira/browse/PIG-682
> Project: Pig
> Issue Type: Bug
> Components: impl
> Reporter: Benjamin Reed
> Attachments: jsch-0.1.41.jar, PIG-682.patch
>
>
> Hadoop has changed a bit and the ssh-gateway code no longer works. pig needs to be updated to register with the new socket framework. reporting of problems also needs to be better.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (PIG-682) Fix the ssh tunneling code
Posted by "Santhosh Srinivasan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-682?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12674378#action_12674378 ]
Santhosh Srinivasan commented on PIG-682:
-----------------------------------------
Review comments:
General note:
1. The list of error codes and associated strings are documented on the wiki at:
http://wiki.apache.org/pig/PigErrorHandlingFunctionalSpecification
Look for the section titled "Compendium of error messages"
src/org/apache/pig/Main.java
=====================
1. The following code is not required. ExecException is now subclassed from PigException.
+ } catch (ExecException e) {
+ System.err.println("Exec error: " + e.getMessage());
+ rc = 2;
2. We should not be printing stack traces anymore. Exceptions that occur in main (within the execution part are already handled by Grunt.java. I could not think of use cases where there was an unhandled exception within Grunt and we did not report it as such.
+ } catch (Throwable e) {
rc = 2;
+ System.err.println("Unrecoverable error: " + e.getMessage());
+ e.printStackTrace();
Index: lib-src/shock/org/apache/pig/shock/SSHSocketImplFactory.java
===================================================================
1. If its a non-fatal error, log.warn is appropriate and not log.error
public SocketImpl createSocketImpl() {
- return new SSHSocketImpl(session);
-
+ try {
+ return new SSHSocketImpl(session);
+ } catch(Throwable e) {
+ log.error("Couldn't create impl", e);
+ return null;
+ }
}
Index: src/org/apache/pig/backend/hadoop/executionengine/HExecutionEngine.java
===================================================================
The setSSHFactory() method catches the SocketException but does nothing. Is this behaviour expected?
catch (SocketException e) {}
> Fix the ssh tunneling code
> --------------------------
>
> Key: PIG-682
> URL: https://issues.apache.org/jira/browse/PIG-682
> Project: Pig
> Issue Type: Bug
> Components: impl
> Reporter: Benjamin Reed
> Attachments: jsch-0.1.41.jar, PIG-682.patch
>
>
> Hadoop has changed a bit and the ssh-gateway code no longer works. pig needs to be updated to register with the new socket framework. reporting of problems also needs to be better.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (PIG-682) Fix the ssh tunneling code
Posted by "Alan Gates (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-682?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alan Gates updated PIG-682:
---------------------------
Status: Open (was: Patch Available)
Moving to open until the patch is changed per the comments by Santhosh and Pradeep.
> Fix the ssh tunneling code
> --------------------------
>
> Key: PIG-682
> URL: https://issues.apache.org/jira/browse/PIG-682
> Project: Pig
> Issue Type: Bug
> Components: impl
> Reporter: Benjamin Reed
> Attachments: jsch-0.1.41.jar, PIG-682.patch
>
>
> Hadoop has changed a bit and the ssh-gateway code no longer works. pig needs to be updated to register with the new socket framework. reporting of problems also needs to be better.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (PIG-682) Fix the ssh tunneling code
Posted by "Olga Natkovich (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-682?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Olga Natkovich resolved PIG-682.
--------------------------------
Resolution: Won't Fix
We no longer use this code
> Fix the ssh tunneling code
> --------------------------
>
> Key: PIG-682
> URL: https://issues.apache.org/jira/browse/PIG-682
> Project: Pig
> Issue Type: Bug
> Components: impl
> Reporter: Benjamin Reed
> Attachments: jsch-0.1.41.jar, PIG-682.patch
>
>
> Hadoop has changed a bit and the ssh-gateway code no longer works. pig needs to be updated to register with the new socket framework. reporting of problems also needs to be better.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.