architjainjain commented on code in PR #6501:
URL: https://github.com/apache/hive/pull/6501#discussion_r3719223661


##########
ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestTezSessionState.java:
##########
@@ -133,4 +137,122 @@ void openInternalUnsafe(boolean isAsync, 
SessionState.LogHelper console) {
 
     sessionStateForTest.open(resources);
   }
+
+  /**
+   * Tests that YarnClient is NOT initialized when queue metrics are disabled 
(default: interval=0).
+   * This ensures zero overhead when the feature is disabled.
+   */
+  @Test
+  public void testYarnClientNotInitializedWhenMetricsDisabled() {
+    SessionState ss = createSessionState();
+    HiveConf hiveConf = ss.getConf();
+    
+    // Default config: queue metrics disabled (interval = 0)
+    Assert.assertEquals("Default interval should be 0 (disabled)",
+        0, HiveConf.getTimeVar(hiveConf, 
HiveConf.ConfVars.HIVE_TEZ_QUEUE_METRICS_REFRESH_INTERVAL, 
TimeUnit.MILLISECONDS));
+
+    TezSessionState sessionState = new TezSessionState(ss.getSessionId(), 
hiveConf);
+    
+    // Mock a TezClient and set it
+    TezClient mockTezClient = Mockito.mock(TezClient.class);
+    sessionState.setTezClient(mockTezClient);
+    
+    // getYarnClient() should return null when metrics disabled
+    YarnClient yarnClient = sessionState.getYarnClient();
+    Assert.assertNull("YarnClient should not be initialized when queue metrics 
are disabled", yarnClient);
+  }
+
+  /**
+   * Tests that YarnClient IS lazily initialized when queue metrics are 
enabled.
+   * This ensures the client is created only when needed.
+   */
+  @Test
+  public void testYarnClientLazilyInitializedWhenMetricsEnabled() {
+    SessionState ss = createSessionState();
+    HiveConf hiveConf = ss.getConf();
+    
+    // Enable queue metrics with a positive interval
+    
hiveConf.setTimeVar(HiveConf.ConfVars.HIVE_TEZ_QUEUE_METRICS_REFRESH_INTERVAL, 
10, TimeUnit.SECONDS);
+
+    TezSessionState sessionState = new TezSessionState(ss.getSessionId(), 
hiveConf);
+    
+    // Mock a TezClient and set it
+    TezClient mockTezClient = Mockito.mock(TezClient.class);
+    sessionState.setTezClient(mockTezClient);
+    
+    // First call to getYarnClient() should initialize it
+    YarnClient yarnClient = sessionState.getYarnClient();
+    Assert.assertNotNull("YarnClient should be initialized when queue metrics 
are enabled", yarnClient);
+    
+    // Second call should return the same instance
+    YarnClient yarnClient2 = sessionState.getYarnClient();
+    Assert.assertSame("Should return the same YarnClient instance", 
yarnClient, yarnClient2);
+  }
+
+  /**
+   * Tests that YarnClient is not initialized when TezClient is null,
+   * even if queue metrics are enabled.
+   */
+  @Test
+  public void testYarnClientNotInitializedWhenTezClientNull() {
+    SessionState ss = createSessionState();
+    HiveConf hiveConf = ss.getConf();
+    
+    // Enable queue metrics
+    
hiveConf.setTimeVar(HiveConf.ConfVars.HIVE_TEZ_QUEUE_METRICS_REFRESH_INTERVAL, 
10, TimeUnit.SECONDS);
+
+    TezSessionState sessionState = new TezSessionState(ss.getSessionId(), 
hiveConf);
+    
+    // Don't set TezClient (session is null)
+    
+    // getYarnClient() should return null when TezClient is not set
+    YarnClient yarnClient = sessionState.getYarnClient();
+    Assert.assertNull("YarnClient should not be initialized when TezClient is 
null", yarnClient);
+  }
+
+  /**
+   * Tests the thread-safety of lazy YarnClient initialization with concurrent 
calls.
+   */
+  @Test
+  public void testYarnClientLazyInitializationThreadSafety() throws 
InterruptedException {
+    SessionState ss = createSessionState();
+    HiveConf hiveConf = ss.getConf();
+    
+    // Enable queue metrics
+    
hiveConf.setTimeVar(HiveConf.ConfVars.HIVE_TEZ_QUEUE_METRICS_REFRESH_INTERVAL, 
10, TimeUnit.SECONDS);
+
+    TezSessionState sessionState = new TezSessionState(ss.getSessionId(), 
hiveConf);
+    TezClient mockTezClient = Mockito.mock(TezClient.class);
+    sessionState.setTezClient(mockTezClient);
+    
+    // Create multiple threads that call getYarnClient() concurrently
+    final int threadCount = 10;
+    Thread[] threads = new Thread[threadCount];
+    YarnClient[] clients = new YarnClient[threadCount];
+    
+    for (int i = 0; i < threadCount; i++) {
+      final int index = i;
+      threads[i] = new Thread(() -> {
+        clients[index] = sessionState.getYarnClient();
+      });
+    }
+    
+    // Start all threads
+    for (Thread thread : threads) {
+      thread.start();
+    }
+    
+    // Wait for all threads to complete
+    for (Thread thread : threads) {
+      thread.join();
+    }
+    
+    // All threads should get the same YarnClient instance
+    YarnClient firstClient = clients[0];
+    Assert.assertNotNull("YarnClient should be initialized", firstClient);
+    
+    for (int i = 1; i < threadCount; i++) {
+      Assert.assertSame("All threads should get the same YarnClient instance", 
firstClient, clients[i]);
+    }
+  }

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to