From 52b60191f11e243fd98433463fc727fea2ff7e0e Mon Sep 17 00:00:00 2001 From: Sai Shree Pradhan Date: Thu, 19 Jun 2025 14:28:11 +0530 Subject: [PATCH 01/10] removed walrus, removed random test before stalling test Signed-off-by: Sai Shree Pradhan --- .github/workflows/code-quality-checks.yml | 2 +- .../sql/telemetry/telemetry_client.py | 12 ++-- tests/unit/test_client.py | 61 ++++++++++--------- 3 files changed, 39 insertions(+), 36 deletions(-) diff --git a/.github/workflows/code-quality-checks.yml b/.github/workflows/code-quality-checks.yml index 462d22369..244118195 100644 --- a/.github/workflows/code-quality-checks.yml +++ b/.github/workflows/code-quality-checks.yml @@ -112,7 +112,7 @@ jobs: # run test suite #---------------------------------------------- - name: Run tests - run: poetry run python -m pytest tests/unit + run: poetry run python -m pytest tests/unit -v -s check-linting: runs-on: ubuntu-latest strategy: diff --git a/src/databricks/sql/telemetry/telemetry_client.py b/src/databricks/sql/telemetry/telemetry_client.py index f7fccf47a..452736086 100644 --- a/src/databricks/sql/telemetry/telemetry_client.py +++ b/src/databricks/sql/telemetry/telemetry_client.py @@ -415,14 +415,16 @@ def close(session_id_hex): """Close and remove the telemetry client for a specific connection""" with TelemetryClientFactory._lock: - if ( - telemetry_client := TelemetryClientFactory._clients.pop( - session_id_hex, None - ) - ) is not None: + # if ( + # telemetry_client := TelemetryClientFactory._clients.pop( + # session_id_hex, None + # ) + # ) is not None: + if session_id_hex in TelemetryClientFactory._clients: logger.debug( "Removing telemetry client for connection %s", session_id_hex ) + telemetry_client = TelemetryClientFactory._clients.pop(session_id_hex) telemetry_client.close() # Shutdown executor if no more clients diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 588b0d70e..cb4058b92 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -353,6 +353,7 @@ def test_context_manager_closes_cursor(self): @patch("%s.client.ThriftBackend" % PACKAGE_NAME) def test_context_manager_closes_connection(self, mock_client_class): + print("stalling test") instance = mock_client_class.return_value mock_open_session_resp = MagicMock(spec=TOpenSessionResp)() @@ -649,36 +650,36 @@ def test_disable_pandas_respected(self, mock_thrift_backend_class): mock_table.itercolumns.assert_called_once_with() - def test_column_name_api(self): - ResultRow = Row("first_col", "second_col", "third_col") - data = [ - ResultRow("val1", 321, 52.32), - ResultRow("val2", 2321, 252.32), - ] - - expected_values = [["val1", 321, 52.32], ["val2", 2321, 252.32]] - - for (row, expected) in zip(data, expected_values): - self.assertEqual(row.first_col, expected[0]) - self.assertEqual(row.second_col, expected[1]) - self.assertEqual(row.third_col, expected[2]) - - self.assertEqual(row["first_col"], expected[0]) - self.assertEqual(row["second_col"], expected[1]) - self.assertEqual(row["third_col"], expected[2]) - - self.assertEqual(row[0], expected[0]) - self.assertEqual(row[1], expected[1]) - self.assertEqual(row[2], expected[2]) - - self.assertEqual( - row.asDict(), - { - "first_col": expected[0], - "second_col": expected[1], - "third_col": expected[2], - }, - ) + # def test_column_name_api(self): + # ResultRow = Row("first_col", "second_col", "third_col") + # data = [ + # ResultRow("val1", 321, 52.32), + # ResultRow("val2", 2321, 252.32), + # ] + + # expected_values = [["val1", 321, 52.32], ["val2", 2321, 252.32]] + + # for (row, expected) in zip(data, expected_values): + # self.assertEqual(row.first_col, expected[0]) + # self.assertEqual(row.second_col, expected[1]) + # self.assertEqual(row.third_col, expected[2]) + + # self.assertEqual(row["first_col"], expected[0]) + # self.assertEqual(row["second_col"], expected[1]) + # self.assertEqual(row["third_col"], expected[2]) + + # self.assertEqual(row[0], expected[0]) + # self.assertEqual(row[1], expected[1]) + # self.assertEqual(row[2], expected[2]) + + # self.assertEqual( + # row.asDict(), + # { + # "first_col": expected[0], + # "second_col": expected[1], + # "third_col": expected[2], + # }, + # ) @patch("%s.client.ThriftBackend" % PACKAGE_NAME) def test_finalizer_closes_abandoned_connection(self, mock_client_class): From 1e5f2eb876d96877c3421072a705540f6637114a Mon Sep 17 00:00:00 2001 From: Sai Shree Pradhan Date: Sat, 21 Jun 2025 09:20:19 +0530 Subject: [PATCH 02/10] added back random test, connection debug Signed-off-by: Sai Shree Pradhan --- src/databricks/sql/client.py | 3 +- tests/unit/test_client.py | 60 ++++++++++++++++++------------------ 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/src/databricks/sql/client.py b/src/databricks/sql/client.py index 26705f3f8..be9502d38 100755 --- a/src/databricks/sql/client.py +++ b/src/databricks/sql/client.py @@ -305,7 +305,7 @@ def read(self) -> Optional[OAuthToken]: self.use_inline_params = self._set_use_inline_params_with_warning( kwargs.get("use_inline_params", False) ) - + print("Connection init : session_id_hex", self.get_session_id_hex(), flush=True) TelemetryClientFactory.initialize_telemetry_client( telemetry_enabled=self.telemetry_enabled, session_id_hex=self.get_session_id_hex(), @@ -440,6 +440,7 @@ def cursor( def close(self) -> None: """Close the underlying session and mark all associated cursors as closed.""" + print("Connection close: session_id_hex: ", self.get_session_id_hex(), flush=True) self._close() def _close(self, close_cursors=True) -> None: diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index cb4058b92..3f16d3341 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -650,36 +650,36 @@ def test_disable_pandas_respected(self, mock_thrift_backend_class): mock_table.itercolumns.assert_called_once_with() - # def test_column_name_api(self): - # ResultRow = Row("first_col", "second_col", "third_col") - # data = [ - # ResultRow("val1", 321, 52.32), - # ResultRow("val2", 2321, 252.32), - # ] - - # expected_values = [["val1", 321, 52.32], ["val2", 2321, 252.32]] - - # for (row, expected) in zip(data, expected_values): - # self.assertEqual(row.first_col, expected[0]) - # self.assertEqual(row.second_col, expected[1]) - # self.assertEqual(row.third_col, expected[2]) - - # self.assertEqual(row["first_col"], expected[0]) - # self.assertEqual(row["second_col"], expected[1]) - # self.assertEqual(row["third_col"], expected[2]) - - # self.assertEqual(row[0], expected[0]) - # self.assertEqual(row[1], expected[1]) - # self.assertEqual(row[2], expected[2]) - - # self.assertEqual( - # row.asDict(), - # { - # "first_col": expected[0], - # "second_col": expected[1], - # "third_col": expected[2], - # }, - # ) + def test_column_name_api(self): + ResultRow = Row("first_col", "second_col", "third_col") + data = [ + ResultRow("val1", 321, 52.32), + ResultRow("val2", 2321, 252.32), + ] + + expected_values = [["val1", 321, 52.32], ["val2", 2321, 252.32]] + + for (row, expected) in zip(data, expected_values): + self.assertEqual(row.first_col, expected[0]) + self.assertEqual(row.second_col, expected[1]) + self.assertEqual(row.third_col, expected[2]) + + self.assertEqual(row["first_col"], expected[0]) + self.assertEqual(row["second_col"], expected[1]) + self.assertEqual(row["third_col"], expected[2]) + + self.assertEqual(row[0], expected[0]) + self.assertEqual(row[1], expected[1]) + self.assertEqual(row[2], expected[2]) + + self.assertEqual( + row.asDict(), + { + "first_col": expected[0], + "second_col": expected[1], + "third_col": expected[2], + }, + ) @patch("%s.client.ThriftBackend" % PACKAGE_NAME) def test_finalizer_closes_abandoned_connection(self, mock_client_class): From 76a74d5c56d91f16515b838b16464264ac7eccb0 Mon Sep 17 00:00:00 2001 From: Sai Shree Pradhan Date: Sat, 21 Jun 2025 09:21:39 +0530 Subject: [PATCH 03/10] formatting Signed-off-by: Sai Shree Pradhan --- .github/workflows/code-quality-checks.yml | 2 +- src/databricks/sql/client.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/code-quality-checks.yml b/.github/workflows/code-quality-checks.yml index 244118195..36bfa1bb0 100644 --- a/.github/workflows/code-quality-checks.yml +++ b/.github/workflows/code-quality-checks.yml @@ -61,7 +61,7 @@ jobs: # run test suite #---------------------------------------------- - name: Run tests - run: poetry run python -m pytest tests/unit + run: poetry run python -m pytest tests/unit -v -s run-unit-tests-with-arrow: runs-on: ubuntu-latest strategy: diff --git a/src/databricks/sql/client.py b/src/databricks/sql/client.py index be9502d38..cacf0ddb4 100755 --- a/src/databricks/sql/client.py +++ b/src/databricks/sql/client.py @@ -440,7 +440,9 @@ def cursor( def close(self) -> None: """Close the underlying session and mark all associated cursors as closed.""" - print("Connection close: session_id_hex: ", self.get_session_id_hex(), flush=True) + print( + "Connection close: session_id_hex: ", self.get_session_id_hex(), flush=True + ) self._close() def _close(self, close_cursors=True) -> None: From 53636253583a027cc6b0ad3081b7074e858a3ba9 Mon Sep 17 00:00:00 2001 From: Sai Shree Pradhan Date: Sat, 21 Jun 2025 09:31:46 +0530 Subject: [PATCH 04/10] telemetry client factory debug Signed-off-by: Sai Shree Pradhan --- .../sql/telemetry/telemetry_client.py | 68 ++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/src/databricks/sql/telemetry/telemetry_client.py b/src/databricks/sql/telemetry/telemetry_client.py index 452736086..2a0b170be 100644 --- a/src/databricks/sql/telemetry/telemetry_client.py +++ b/src/databricks/sql/telemetry/telemetry_client.py @@ -367,15 +367,36 @@ def initialize_telemetry_client( """Initialize a telemetry client for a specific connection if telemetry is enabled""" try: + print( + "\nWAITING: Initializing telemetry client: %s", + session_id_hex, + flush=True, + ) with TelemetryClientFactory._lock: + print( + "\nACQUIRED: Initializing telemetry client, got lock: %s", + session_id_hex, + flush=True, + ) TelemetryClientFactory._initialize() + print( + "\n TelemetryClientFactory initialized: %s", + session_id_hex, + flush=True, + ) if session_id_hex not in TelemetryClientFactory._clients: + print( + "\n Session ID not in clients: %s", + session_id_hex, + flush=True, + ) logger.debug( "Creating new TelemetryClient for connection %s", session_id_hex, ) if telemetry_enabled: + print("\n Telemetry enabled: %s", session_id_hex, flush=True) TelemetryClientFactory._clients[ session_id_hex ] = TelemetryClient( @@ -385,11 +406,41 @@ def initialize_telemetry_client( host_url=host_url, executor=TelemetryClientFactory._executor, ) + print( + "\n Telemetry client initialized: %s", + session_id_hex, + flush=True, + ) else: + print( + "\n Telemetry disabled: %s", session_id_hex, flush=True + ) TelemetryClientFactory._clients[ session_id_hex ] = NoopTelemetryClient() + print( + "\n Noop Telemetry client initialized: %s", + session_id_hex, + flush=True, + ) + else: + print( + "\n Session ID already in clients: %s", + session_id_hex, + flush=True, + ) + print( + "\nRELEASED: Telemetry client initialized: %s", + session_id_hex, + flush=True, + ) except Exception as e: + print( + "\nERROR: Failed to initialize telemetry client: %s due to %s", + session_id_hex, + e, + flush=True, + ) logger.debug("Failed to initialize telemetry client: %s", e) # Fallback to NoopTelemetryClient to ensure connection doesn't fail TelemetryClientFactory._clients[session_id_hex] = NoopTelemetryClient() @@ -413,8 +464,13 @@ def get_telemetry_client(session_id_hex): @staticmethod def close(session_id_hex): """Close and remove the telemetry client for a specific connection""" - + print("\nWAITING: Closing telemetry client: %s", session_id_hex, flush=True) with TelemetryClientFactory._lock: + print( + "\nACQUIRED: Closing telemetry client, got lock: %s", + session_id_hex, + flush=True, + ) # if ( # telemetry_client := TelemetryClientFactory._clients.pop( # session_id_hex, None @@ -432,6 +488,16 @@ def close(session_id_hex): logger.debug( "No more telemetry clients, shutting down thread pool executor" ) + print( + "\nSHUTDOWN: Shutting down thread pool executor: %s", + session_id_hex, + flush=True, + ) TelemetryClientFactory._executor.shutdown(wait=True) TelemetryClientFactory._executor = None TelemetryClientFactory._initialized = False + print( + "\nRELEASED: Thread pool executor shut down: %s", + session_id_hex, + flush=True, + ) From e94978f04f24825bd1b0eb16a405faf8540040a8 Mon Sep 17 00:00:00 2001 From: Sai Shree Pradhan Date: Sun, 22 Jun 2025 16:30:46 +0530 Subject: [PATCH 05/10] garbage collector Signed-off-by: Sai Shree Pradhan --- src/databricks/sql/client.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/databricks/sql/client.py b/src/databricks/sql/client.py index cacf0ddb4..8ab326f1f 100755 --- a/src/databricks/sql/client.py +++ b/src/databricks/sql/client.py @@ -377,6 +377,11 @@ def __del__(self): "Closing unclosed connection for session " "{}".format(self.get_session_id_hex()) ) + print( + "LMAO GC closing connection IN MAIN THREAD (INTERRUPTS EVERYTHING): ", + self.get_session_id_hex(), + flush=True, + ) try: self._close(close_cursors=False) except OperationalError as e: From deef881eabce1364d4084349a198f534fa37a071 Mon Sep 17 00:00:00 2001 From: Sai Shree Pradhan Date: Sun, 22 Jun 2025 16:34:04 +0530 Subject: [PATCH 06/10] RLOCK IS THE SOLUTION Signed-off-by: Sai Shree Pradhan --- src/databricks/sql/telemetry/telemetry_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/databricks/sql/telemetry/telemetry_client.py b/src/databricks/sql/telemetry/telemetry_client.py index 2a0b170be..10d3ed54d 100644 --- a/src/databricks/sql/telemetry/telemetry_client.py +++ b/src/databricks/sql/telemetry/telemetry_client.py @@ -316,7 +316,7 @@ class TelemetryClientFactory: ] = {} # Map of session_id_hex -> BaseTelemetryClient _executor: Optional[ThreadPoolExecutor] = None _initialized: bool = False - _lock = threading.Lock() # Thread safety for factory operations + _lock = threading.RLock() # Thread safety for factory operations _original_excepthook = None _excepthook_installed = False From 6fe369fa2a017ebc1e0c510af294bb4881c2625d Mon Sep 17 00:00:00 2001 From: Sai Shree Pradhan Date: Mon, 23 Jun 2025 10:01:39 +0530 Subject: [PATCH 07/10] removed debug statements Signed-off-by: Sai Shree Pradhan --- .github/workflows/code-quality-checks.yml | 4 +- src/databricks/sql/client.py | 10 +-- .../sql/telemetry/telemetry_client.py | 82 +++---------------- tests/unit/test_client.py | 1 - 4 files changed, 15 insertions(+), 82 deletions(-) diff --git a/.github/workflows/code-quality-checks.yml b/.github/workflows/code-quality-checks.yml index 36bfa1bb0..462d22369 100644 --- a/.github/workflows/code-quality-checks.yml +++ b/.github/workflows/code-quality-checks.yml @@ -61,7 +61,7 @@ jobs: # run test suite #---------------------------------------------- - name: Run tests - run: poetry run python -m pytest tests/unit -v -s + run: poetry run python -m pytest tests/unit run-unit-tests-with-arrow: runs-on: ubuntu-latest strategy: @@ -112,7 +112,7 @@ jobs: # run test suite #---------------------------------------------- - name: Run tests - run: poetry run python -m pytest tests/unit -v -s + run: poetry run python -m pytest tests/unit check-linting: runs-on: ubuntu-latest strategy: diff --git a/src/databricks/sql/client.py b/src/databricks/sql/client.py index 8ab326f1f..26705f3f8 100755 --- a/src/databricks/sql/client.py +++ b/src/databricks/sql/client.py @@ -305,7 +305,7 @@ def read(self) -> Optional[OAuthToken]: self.use_inline_params = self._set_use_inline_params_with_warning( kwargs.get("use_inline_params", False) ) - print("Connection init : session_id_hex", self.get_session_id_hex(), flush=True) + TelemetryClientFactory.initialize_telemetry_client( telemetry_enabled=self.telemetry_enabled, session_id_hex=self.get_session_id_hex(), @@ -377,11 +377,6 @@ def __del__(self): "Closing unclosed connection for session " "{}".format(self.get_session_id_hex()) ) - print( - "LMAO GC closing connection IN MAIN THREAD (INTERRUPTS EVERYTHING): ", - self.get_session_id_hex(), - flush=True, - ) try: self._close(close_cursors=False) except OperationalError as e: @@ -445,9 +440,6 @@ def cursor( def close(self) -> None: """Close the underlying session and mark all associated cursors as closed.""" - print( - "Connection close: session_id_hex: ", self.get_session_id_hex(), flush=True - ) self._close() def _close(self, close_cursors=True) -> None: diff --git a/src/databricks/sql/telemetry/telemetry_client.py b/src/databricks/sql/telemetry/telemetry_client.py index 10d3ed54d..9cb0497c2 100644 --- a/src/databricks/sql/telemetry/telemetry_client.py +++ b/src/databricks/sql/telemetry/telemetry_client.py @@ -367,27 +367,12 @@ def initialize_telemetry_client( """Initialize a telemetry client for a specific connection if telemetry is enabled""" try: - print( - "\nWAITING: Initializing telemetry client: %s", - session_id_hex, - flush=True, - ) with TelemetryClientFactory._lock: - print( - "\nACQUIRED: Initializing telemetry client, got lock: %s", - session_id_hex, - flush=True, - ) - TelemetryClientFactory._initialize() - print( - "\n TelemetryClientFactory initialized: %s", - session_id_hex, - flush=True, - ) + TelemetryClientFactory._initialize() if session_id_hex not in TelemetryClientFactory._clients: - print( - "\n Session ID not in clients: %s", + logger.debug( + "Session ID not in clients: %s", session_id_hex, flush=True, ) @@ -396,7 +381,6 @@ def initialize_telemetry_client( session_id_hex, ) if telemetry_enabled: - print("\n Telemetry enabled: %s", session_id_hex, flush=True) TelemetryClientFactory._clients[ session_id_hex ] = TelemetryClient( @@ -406,41 +390,16 @@ def initialize_telemetry_client( host_url=host_url, executor=TelemetryClientFactory._executor, ) - print( - "\n Telemetry client initialized: %s", - session_id_hex, - flush=True, - ) else: - print( - "\n Telemetry disabled: %s", session_id_hex, flush=True - ) + logger.debug("Telemetry disabled: %s", session_id_hex) TelemetryClientFactory._clients[ session_id_hex ] = NoopTelemetryClient() - print( - "\n Noop Telemetry client initialized: %s", - session_id_hex, - flush=True, + logger.debug( + "Noop Telemetry client initialized: %s", session_id_hex ) - else: - print( - "\n Session ID already in clients: %s", - session_id_hex, - flush=True, - ) - print( - "\nRELEASED: Telemetry client initialized: %s", - session_id_hex, - flush=True, - ) + except Exception as e: - print( - "\nERROR: Failed to initialize telemetry client: %s due to %s", - session_id_hex, - e, - flush=True, - ) logger.debug("Failed to initialize telemetry client: %s", e) # Fallback to NoopTelemetryClient to ensure connection doesn't fail TelemetryClientFactory._clients[session_id_hex] = NoopTelemetryClient() @@ -464,19 +423,12 @@ def get_telemetry_client(session_id_hex): @staticmethod def close(session_id_hex): """Close and remove the telemetry client for a specific connection""" - print("\nWAITING: Closing telemetry client: %s", session_id_hex, flush=True) with TelemetryClientFactory._lock: - print( - "\nACQUIRED: Closing telemetry client, got lock: %s", - session_id_hex, - flush=True, - ) - # if ( - # telemetry_client := TelemetryClientFactory._clients.pop( - # session_id_hex, None - # ) - # ) is not None: - if session_id_hex in TelemetryClientFactory._clients: + if ( + telemetry_client := TelemetryClientFactory._clients.pop( + session_id_hex, None + ) + ) is not None: logger.debug( "Removing telemetry client for connection %s", session_id_hex ) @@ -488,16 +440,6 @@ def close(session_id_hex): logger.debug( "No more telemetry clients, shutting down thread pool executor" ) - print( - "\nSHUTDOWN: Shutting down thread pool executor: %s", - session_id_hex, - flush=True, - ) TelemetryClientFactory._executor.shutdown(wait=True) TelemetryClientFactory._executor = None TelemetryClientFactory._initialized = False - print( - "\nRELEASED: Thread pool executor shut down: %s", - session_id_hex, - flush=True, - ) diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 3f16d3341..588b0d70e 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -353,7 +353,6 @@ def test_context_manager_closes_cursor(self): @patch("%s.client.ThriftBackend" % PACKAGE_NAME) def test_context_manager_closes_connection(self, mock_client_class): - print("stalling test") instance = mock_client_class.return_value mock_open_session_resp = MagicMock(spec=TOpenSessionResp)() From e3b64ee72311a5f1d8d7c8dad1ef408467bc03eb Mon Sep 17 00:00:00 2001 From: Sai Shree Pradhan Date: Mon, 23 Jun 2025 10:03:46 +0530 Subject: [PATCH 08/10] remove debugs Signed-off-by: Sai Shree Pradhan --- src/databricks/sql/telemetry/telemetry_client.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/databricks/sql/telemetry/telemetry_client.py b/src/databricks/sql/telemetry/telemetry_client.py index 9cb0497c2..294bb78a4 100644 --- a/src/databricks/sql/telemetry/telemetry_client.py +++ b/src/databricks/sql/telemetry/telemetry_client.py @@ -368,14 +368,8 @@ def initialize_telemetry_client( try: with TelemetryClientFactory._lock: - TelemetryClientFactory._initialize() if session_id_hex not in TelemetryClientFactory._clients: - logger.debug( - "Session ID not in clients: %s", - session_id_hex, - flush=True, - ) logger.debug( "Creating new TelemetryClient for connection %s", session_id_hex, @@ -391,13 +385,9 @@ def initialize_telemetry_client( executor=TelemetryClientFactory._executor, ) else: - logger.debug("Telemetry disabled: %s", session_id_hex) TelemetryClientFactory._clients[ session_id_hex ] = NoopTelemetryClient() - logger.debug( - "Noop Telemetry client initialized: %s", session_id_hex - ) except Exception as e: logger.debug("Failed to initialize telemetry client: %s", e) @@ -423,6 +413,7 @@ def get_telemetry_client(session_id_hex): @staticmethod def close(session_id_hex): """Close and remove the telemetry client for a specific connection""" + with TelemetryClientFactory._lock: if ( telemetry_client := TelemetryClientFactory._clients.pop( @@ -432,7 +423,6 @@ def close(session_id_hex): logger.debug( "Removing telemetry client for connection %s", session_id_hex ) - telemetry_client = TelemetryClientFactory._clients.pop(session_id_hex) telemetry_client.close() # Shutdown executor if no more clients From 7ca12a408dd621dc3c453866a2649ce7086c43ab Mon Sep 17 00:00:00 2001 From: Sai Shree Pradhan Date: Mon, 23 Jun 2025 10:04:57 +0530 Subject: [PATCH 09/10] removed debug Signed-off-by: Sai Shree Pradhan --- src/databricks/sql/telemetry/telemetry_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/databricks/sql/telemetry/telemetry_client.py b/src/databricks/sql/telemetry/telemetry_client.py index 294bb78a4..6b964e23b 100644 --- a/src/databricks/sql/telemetry/telemetry_client.py +++ b/src/databricks/sql/telemetry/telemetry_client.py @@ -369,6 +369,7 @@ def initialize_telemetry_client( with TelemetryClientFactory._lock: TelemetryClientFactory._initialize() + if session_id_hex not in TelemetryClientFactory._clients: logger.debug( "Creating new TelemetryClient for connection %s", @@ -388,7 +389,6 @@ def initialize_telemetry_client( TelemetryClientFactory._clients[ session_id_hex ] = NoopTelemetryClient() - except Exception as e: logger.debug("Failed to initialize telemetry client: %s", e) # Fallback to NoopTelemetryClient to ensure connection doesn't fail From 095491ae06bd67299e6630bb7d7e2b64e061492a Mon Sep 17 00:00:00 2001 From: Sai Shree Pradhan Date: Mon, 23 Jun 2025 11:24:30 +0530 Subject: [PATCH 10/10] added comment for RLock Signed-off-by: Sai Shree Pradhan --- src/databricks/sql/telemetry/telemetry_client.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/databricks/sql/telemetry/telemetry_client.py b/src/databricks/sql/telemetry/telemetry_client.py index 6b964e23b..10aa04eff 100644 --- a/src/databricks/sql/telemetry/telemetry_client.py +++ b/src/databricks/sql/telemetry/telemetry_client.py @@ -317,6 +317,7 @@ class TelemetryClientFactory: _executor: Optional[ThreadPoolExecutor] = None _initialized: bool = False _lock = threading.RLock() # Thread safety for factory operations + # used RLock instead of Lock to avoid deadlocks when garbage collection is triggered _original_excepthook = None _excepthook_installed = False