[ 
https://issues.apache.org/jira/browse/HBASE-26469?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17455992#comment-17455992
 ] 

Sean Busbey edited comment on HBASE-26469 at 12/8/21, 9:28 PM:
---------------------------------------------------------------

{quote}What is the intent here? For 2.4 to match what 2.3 did (despite some of 
the behavior being arguably wrong?) Or to correct some of the behavior now that 
we know about it?
{quote}
I would like to correct the behavior now that we know about it to match what is 
documented (in the cli help and ref guide as of HBASE-11658 and HBASE-11655. I 
think probably this would be fine with a release note for 2.5+.

I am hesitant to change exit codes in patch releases. I think the additional 
error indicating exit codes that happen on 2.4.y are disruptive enough for 
those migrating from 2.3 and earlier that branch-2.4 should be updated to match 
the behavior in 2.3. (edited for clarity, thanks Mike)

 
{quote}I don't remember the conventions, but maybe cli input should always be 
non-interactive? I am confident that we've had these conversations in the past.
{quote}
I don't remember where the conversation happened but the `–-noninteractive` was 
expressly opt-in to avoid folks getting blown up in operations who didn't 
expect the shell to return non-zero basically ever. Maybe we can revisit that 
in a follow-on for hbase 3?


was (Author: busbey):
{quote}What is the intent here? For 2.4 to match what 2.3 did (despite some of 
the behavior being arguably wrong?) Or to correct some of the behavior now that 
we know about it?
{quote}
I would like to correct the behavior now that we know about it to match what is 
documented (in the cli help and ref guide as of HBASE-11658 and HBASE-11655. I 
think probably this would be fine with a release note for 2.5+.

I am hesitant to change exit codes in patch releases. I think additional error 
indicating exit codes that happen on 2.4.y are disruptive enough for those 
migrating from 2.3 and earlier that branch-2.4 should be updated to match the 
behavior in 2.3. (edited for clarity, thanks Mike)

 
{quote}I don't remember the conventions, but maybe cli input should always be 
non-interactive? I am confident that we've had these conversations in the past.
{quote}
I don't remember where the conversation happened but the `–-noninteractive` was 
expressly opt-in to avoid folks getting blown up in operations who didn't 
expect the shell to return non-zero basically ever. Maybe we can revisit that 
in a follow-on for hbase 3?

> HBase shell has changed exit behavior
> -------------------------------------
>
>                 Key: HBASE-26469
>                 URL: https://issues.apache.org/jira/browse/HBASE-26469
>             Project: HBase
>          Issue Type: Bug
>          Components: shell
>    Affects Versions: 2.5.0, 3.0.0-alpha-2, 2.4.8
>            Reporter: Sean Busbey
>            Assignee: Sean Busbey
>            Priority: Critical
>         Attachments: hbase-1.4.14-exit-behavior.log, 
> hbase-1.7.1-exit-behavior.log, hbase-2.0.6-exit-behavior.log, 
> hbase-2.1.9-exit-behavior.log, hbase-2.2.7-exit-behavior.log, 
> hbase-2.3.7-exit-behavior.log, hbase-2.4.8-exit-behavior.log, 
> hbase-3.0.0-alpha-2-exit-behavior.log
>
>
> The HBase shell has changed behavior in a way that breaks being able to exit 
> properly.
> Two example scripts to act as stand ins for hbase shell scripts to "do 
> something simple then exit":
> {code}
> tmp % echo "list\nexit" > clean_exit.rb
> tmp % echo "list\nexit 1" > error_exit.rb
> {code}
> Giving these two scripts is possible:
> * passed as a cli argument
> * via redirected stdin
> Additionally the shell invocation can be:
> * in the default compatibility mode
> * with the "non interactive" flag that gives an exit code that reflects 
> runtime errors
> I'll post logs of the details as attachments but here are some tables of the 
> exit codes.
> The {{clean_exit.rb}} invocations ought to exit with success, exit code 0.
> || ||          1.4.14 || 1.7.1 || 2.0.6 || 2.1.9 || 2.2.7 || 2.3.7 || 2.4.8 
> || master ||
> | cli, default |    0 |    0   |    0   |    0   |    0   |    0   |    1   | 
>    1*   |
> | cli, -n |         0 |    0   |    0   |    0   |    0   |    0   |    1   | 
>  hang   |
> | stdin, default |  0 |    0   |    0   |    0   |    0   |    0   |    0   | 
>    0    |
> | stdin, -n |       1 |    1   |    1   |    1   |    1   |    1   |    1*  | 
>    1*   |
> The {{error_exit.rb}} invocation should return a non-zero exit code, unless 
> we're specifically trying to match a normal hbase shell session.
> || ||         1.4.14 || 1.7.1 || 2.0.6 || 2.1.9 || 2.2.7 || 2.3.7 || 2.4.8 || 
> master ||
> | cli, default |   1 |    1   |    1   |    1   |    1   |    1   |    1*  |  
>   1*   |
> | cli, -n |        1 |    1   |    1   |    1   |    1   |    1   |    1*  |  
> hang   |
> | stdin, default | 0 |    0   |    0   |    0   |    0   |    0   |    0   |  
>   0    |
> | stdin, -n |      1 |    1   |    1   |    1   |    1   |    1   |    1*  |  
>   1*   |
> In cases marked with * the error details are different.
> The biggest concern are the new-to-2.4 non-zero exit code when we should have 
> a success and the hanging.
> The former looks like this:
> {code}
> ERROR NoMethodError: private method `exit' called for nil:NilClass
> {code}
> The change in error details for the error exit script also shows this same 
> detail.
> This behavior appears to be a side effect of HBASE-11686. As far as I can 
> tell, the IRB handling of 'exit' calls fail because we implement our own 
> handling of sessoins rather than rely on the intended session interface. We 
> never set a current session, and IRB's exit implementation presumes there 
> will be one.
> Running in debug shows this in a stacktrace:
> {code}
> Took 0.4563 seconds
> ERROR NoMethodError: private method `exit' called for nil:NilClass
> NoMethodError: private method `exit' called for nil:NilClass
>                      irb_exit at 
> uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/irb/extend-command.rb:30
>                      evaluate at stdin:2
>                          eval at org/jruby/RubyKernel.java:1048
>                      evaluate at 
> uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/irb/workspace.rb:85
>                       eval_io at uri:classloader:/shell.rb:327
>      each_top_level_statement at 
> uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/irb/ruby-lex.rb:246
>                          loop at org/jruby/RubyKernel.java:1442
>      each_top_level_statement at 
> uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/irb/ruby-lex.rb:232
>                         catch at org/jruby/RubyKernel.java:1189
>      each_top_level_statement at 
> uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/irb/ruby-lex.rb:231
>                       eval_io at uri:classloader:/shell.rb:326
>   classpath:/jar-bootstrap.rb at classpath:/jar-bootstrap.rb:194
>             exception_handler at uri:classloader:/shell.rb:339
>                        <main> at classpath:/jar-bootstrap.rb:194
> {code}
> And in our version of IRB (0.9.6) [line 30 for 
> extend-commands|https://github.com/ruby/irb/blob/v0.9.6/lib/irb/extend-command.rb#L30]
>  corresponds to this code:
> {code}
>     # Quits the current irb context
>     #
>     # +ret+ is the optional signal or message to send to Context#exit
>     #
>     # Same as <code>IRB.CurrentContext.exit</code>.
>     def irb_exit(ret = 0)
>       irb_context.exit(ret)
>     end
> {code}



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to