Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CALCITE-4152] Switch to ConfigurableSpnego Jetty implementations #132

Merged
merged 1 commit into from
Oct 29, 2021
Merged

[CALCITE-4152] Switch to ConfigurableSpnego Jetty implementations #132

merged 1 commit into from
Oct 29, 2021

Conversation

joshelser
Copy link
Member

The new ConfigurableSpnego* classes in Jetty give us the ability to use the SessionHandler to send back a session cookie and avoid having to execute the spnego handshake for each and every call. This is a big improvement in reducing the traffic to the avatica server.

Still a draft

@joshelser joshelser requested a review from risdenk January 1, 2021 04:39
@joshelser
Copy link
Member Author

Just updated this pull request. Getting closer, but still not there.

YCSB test results were very promising. In a nutshell, doing ycsb load against Avatica with hsqldb showed roughly 2x performance improvement (execution time) with a batch size of 5 rows per executeUpdate. This is because we're actually making about half the number of HTTP calls we were previously making. Similarly, with a large batch size (1000rows/batch), execution time is roughly equivalent with the previous performance. The Java client looks great.

However, when I tried to use the Python client with Spnego, I ran into issues. Specifically, I was seeing a NullPointerException where the result from GSSContext.acceptSecContext (inside Avatica) was returning null. Sadly, the JVM docs don't tell me why this might be and I wasn't able to dig into the JVM source enough to peel back the layers of indirection. As such, this still isn't ready to go.

It would be great to get a set of eyes on this now as there is some breaking runtime semantics.

  1. "additionalAllowedRealms" is no more. If you're authenticated via Kerberos, you're allowed in.
  2. There is no external Kerberos authentication for the Avatica server allowed anymore. Jetty will always log in as the given principal/keytab.

@joshelser
Copy link
Member Author

Finally got the time to revisit this. Sure enough, the java solution worked out of the box again and the python approach failed.

I started poking around with GDB, which lead to LLDB, which was able to show me:

* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
  * frame #0: 0x00007fff202fe656 libsystem_kernel.dylib`__select + 10
    frame #1: 0x00007fff2b68fbc5 Heimdal`wait_response + 1125
    frame #2: 0x00007fff2b68ebed Heimdal`krb5_sendto_context + 1229
    frame #3: 0x00007fff2b65995c Heimdal`get_cred_kdc + 1404
    frame #4: 0x00007fff2b65b80a Heimdal`get_cred_kdc_address + 346
    frame #5: 0x00007fff2b65a33d Heimdal`get_cred_kdc_referral + 941
    frame #6: 0x00007fff2b659ec4 Heimdal`_krb5_get_cred_kdc_any + 548
    frame #7: 0x00007fff2b63e43b Heimdal`krb5_get_credentials_with_flags + 1467
    frame #8: 0x00007fff2b63de6d Heimdal`krb5_get_credentials + 61
    frame #9: 0x00007fff2d6ddbc1 GSS`gsskrb5_get_creds + 497
    frame #10: 0x00007fff2d6dd84c GSS`init_krb5_auth + 812
    frame #11: 0x00007fff2d6dcef5 GSS`_gsskrb5_init_sec_context + 965
    frame #12: 0x00007fff2d6dc0d9 GSS`gss_init_sec_context + 1209
    frame #13: 0x00007fff2d6e3984 GSS`initiator_approved + 196
    frame #14: 0x00007fff2d6e2f66 GSS`_gss_spnego_indicate_mechtypelist + 294
    frame #15: 0x00007fff2d6e299c GSS`spnego_initial + 556
    frame #16: 0x00007fff2d6e2584 GSS`_gss_spnego_init_sec_context + 292
    frame #17: 0x00007fff2d6dc0d9 GSS`gss_init_sec_context + 1209

Meaning, requests-gssapi was using gssapi which was calling out to the OSX-provided Heimdal libraries instead of the MIT kerberos libraries I intended to be used. I popped open a Linux docker container and the same python code worked immediately.

It seems like something with the Heimdal libraries that get bundled with OSX don't work with the current SPNEGO code in Jetty. I have no interest in trying to debug that :). I'm gonna work on getting this code cleaned up and a real pull request published.

@joshelser
[CALCITE-4152] Upgrade Avatica to use the configurable SPNEGO Jetty i…
8afc50b 
…mplementation

Jetty has deprecated the previously-used version of SPNEGO login code.
This change requires a few other changes to adopt:

1. Removal of automatic server login via JAAS (Jetty removed this and
    expects explicit logins for the server).
2. Separation of Authentication and Authorization (we're required to
    use a LoginService for authz to use the new SPNEGO authentication).

For the benefit of making this change, we automatically inherit the
Jetty Session logic which can skip SPNEGO authentication for the 2nd
to Nth call to Avatica. For a "workload" which previously took N HTTP calls
to Avatica to perform, this can now be done in (N/2)+1 HTTP calls
which, for average Avatica calls, results in a nearly 2x speed-up.

Jetty Sessions will cause a JSESSIONID cookie to be sent back on the
successful SPNEGO authentication handshake. As long as the client
resubmits this cookie for subsequent requests, the identity of the
client is kept intact.

To test this more easily, this change also includes updates to the
Avatica StandaloneServer, which more easily enables setup of Avatica
against any database (e.g. hsqldb with the SCOTT dataset).
@joshelser joshelser marked this pull request as ready for review October 28, 2021 00:09
@joshelser joshelser requested a review from F21 October 28, 2021 00:09
@joshelser
Copy link
Member Author

Also, fyi @stoty

F21
F21 approved these changes Oct 29, 2021
View reviewed changes
Copy link
Member

@F21 F21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @joshelser !

@joshelser joshelser merged commit c3a9192 into apache:master Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@F21 F21 F21 approved these changes

@risdenk risdenk Awaiting requested review from risdenk

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@joshelser @F21