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.

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

HIVE-28340: Test concurrent JDBC connections with Kerberized cluster, impersonation, and HTTP transport #5315

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

zabetak
Copy link
Contributor

@zabetak zabetak commented Jun 20, 2024

What changes were proposed in this pull request?

  1. New test case simulating JDBC clients concurrently accessing a Kerberized cluster, with impersonation and HTTP transport.
  2. Set transport mode based on configuration when creating MiniHS2 from MiniHiveKdc#getMiniHS2WithKerbWithRemoteHMSWithKerb.

Why are the changes needed?

Increase test coverage and guard against regressions. More info under HIVE-28340.

Does this PR introduce any user-facing change?

No

Is the change a dependency upgrade?

No

How was this patch tested?

mvn test -Dtest=TestJdbcWithMiniKdcDoAsHttp -pl itests/hive-minikdc -Pitests

… impersonation, and HTTP transport

The new test case simulates a scenario with two JDBC clients doing the following in parallel:

client 1, continuously opens and closes connections (short-lived connection)
client 2, opens connection, sends fixed number of simple queries, closes connection (long-lived connection)
Since the clients are running in parallel we have one long-lived session in HS2 interleaved with many short ones.

The test case aims to increase test coverage and guard against regressions in the presence of many interleaved HS2 sessions.

In older versions, without HIVE-27201, this test fails (with the exception outlined below) when the cluster is Kerberized, and we are using HTTP transport mode with impersonation enabled.
// Simulate a client application that uses many short-lived connections.
Future<Boolean> openCloseConnectionTask = service.submit(() -> {
do {
try (Connection c = DriverManager.getConnection(hs2.getJdbcURL())) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm confusing, the hs2.getJdbcURL() should be a Kerberized connection, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, based on the current setup the JDBC URL is:
jdbc:hive2://localhost:33961/default;transportMode=http;httpPath=cliservice;principal=hive/[email protected]

@@ -194,9 +194,6 @@ public static MiniHS2 getMiniHS2WithKerb(MiniHiveKdc miniHiveKdc, HiveConf hiveC
.withConf(hiveConf)
.withMiniKdc(hivePrincipal, hiveKeytab)
.withAuthenticationType(authType);
if (HiveServer2.isHttpTransportMode(hiveConf)) {
Copy link
Member

Choose a reason for hiding this comment

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

why do we remove it here? what if the HS2 reads this property and opens the http server, while the client is trying to establish the connection with the binary port?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The JDBC URL is usually obtained from MiniHS2#getJdbcURL() so the server/client configs should be aligned.

Nevertheless to simplify the review of this PR I reverted the changes here and in MiniHS2 and added the same isHttpTransportMode logic in the the other static factory method that is used in this test (add40a8 ).

…bWithRemoteHMSWithKerb

Leave MiniHS2 and other methods unchanged and mimic other APIs in MiniHiveKdc for setting HTTP mode.
Copy link

sonarcloud bot commented Jun 25, 2024

@zabetak
Copy link
Contributor Author

zabetak commented Jun 26, 2024

Tests are green can you have another look @dengzhhu653 ? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants