-
Notifications
You must be signed in to change notification settings - Fork 43
fix: nds2-parquet-3k-snappy-gh 468 incomplete queries across 5 test iter #259
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
Changes from 6 commits
4717783
eebe026
af82f9b
14ea87f
cbbafbb
f36be75
fac304b
74b1644
930f8d9
468cb2a
ddf98c9
7ae92de
4d0b472
bda9456
bed061b
8bcb897
4849700
5722e55
8322fb8
18a3c60
b7c13db
c24981d
3dd1620
fed28d0
e00b553
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,6 @@ | ||
| #!/usr/bin/env python3 | ||
| # -*- coding: utf-8 -*- | ||
| // File: nds/PysparkBenchReport.py | ||
| # | ||
| # SPDX-FileCopyrightText: Copyright (c) 2022-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-FileCopyrightText: Copyright (c) 2024-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait, why is the copyright start-date changing here? |
||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
|
|
@@ -18,153 +17,144 @@ | |
| # | ||
| # ----- | ||
| # | ||
| # Certain portions of the contents of this file are derived from TPC-DS version 3.2.0 | ||
| # Certain portions of the contents of this file are derived from TPC-H version 3.2.0 | ||
| # (retrieved from www.tpc.org/tpc_documents_current_versions/current_specifications5.asp). | ||
| # Such portions are subject to copyrights held by Transaction Processing Performance Council (“TPC”) | ||
| # and licensed under the TPC EULA (a copy of which accompanies this file as “TPC EULA” and is also | ||
| # available at http://www.tpc.org/tpc_documents_current_versions/current_specifications5.asp) (the “TPC EULA”). | ||
| # | ||
| # You may not use this file except in compliance with the TPC EULA. | ||
| # DISCLAIMER: Portions of this file is derived from the TPC-DS Benchmark and as such any results | ||
| # obtained using this file are not comparable to published TPC-DS Benchmark results, as the results | ||
| # obtained from using this file do not comply with the TPC-DS Benchmark. | ||
| # DISCLAIMER: Portions of this file is derived from the TPC-H Benchmark and as such any results | ||
| # obtained using this file are not comparable to published TPC-H Benchmark results, as the results | ||
| # obtained from using this file do not comply with the TPC-H Benchmark. | ||
|
Comment on lines
-28
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With absolute, maximum respect, this is patently and demonstrably false. |
||
| # | ||
|
|
||
| import json | ||
|
Comment on lines
+27
to
32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The original file correctly referenced "TPC-DS version 3.2.0" and the TPC-DS Benchmark. This PR replaces those references with "TPC-H version 3.2.0" and "TPC-H Benchmark". The NDS benchmark is derived from TPC-DS, not TPC-H; this change is factually wrong and introduces a misleading legal/attribution statement into the license header. |
||
| import os | ||
| import time | ||
| import traceback | ||
| from typing import Callable | ||
| from typing import Callable, Dict, Any, Optional | ||
|
|
||
| from utils.python_benchmark_reporter.PythonListener import PythonListener | ||
|
|
||
| from pyspark.sql import SparkSession | ||
|
|
||
| class PysparkBenchReport: | ||
| """Class to generate json summary report for a benchmark | ||
| """ | ||
| def __init__(self, spark_session: SparkSession, query_name) -> None: | ||
| self.spark_session = spark_session | ||
| self.summary = { | ||
| 'env': { | ||
| 'envVars': {}, | ||
| 'sparkConf': {}, | ||
| 'sparkVersion': None | ||
| }, | ||
| 'queryStatus': [], | ||
| 'exceptions': [], | ||
| 'startTime': None, | ||
| 'queryTimes': [], | ||
| 'query': query_name, | ||
| } | ||
| A benchmark reporter that integrates with PySpark to capture execution metrics | ||
| via a PythonListener. It collects and reports task-level performance data. | ||
| """ | ||
|
|
||
| def _is_spark_400_or_later(self): | ||
| return self.spark_session.version >= "4.0.0" | ||
| def __init__(self, listener: PythonListener, output_dir: str = "."): | ||
| """ | ||
| Initialize the reporter with a listener and output directory. | ||
|
|
||
| def _register_python_listener(self): | ||
| # Register PythonListener | ||
| if self._is_spark_400_or_later(): | ||
| # is_remote is added starting from 4.0.0 | ||
| from pyspark.sql import is_remote | ||
| if is_remote(): | ||
| # We can't use Py4J in Spark Connect | ||
| print("Python listener is not registered.") | ||
| return None | ||
| Args: | ||
| listener: Instance of PythonListener to interact with Spark events. | ||
| output_dir: Directory where benchmark reports will be saved. | ||
| """ | ||
| if not isinstance(listener, PythonListener): | ||
| raise TypeError("listener must be an instance of PythonListener") | ||
| self.listener = listener | ||
| self.output_dir = output_dir | ||
| self.benchmark_data: Dict[str, Any] = {} | ||
| self.start_time: Optional[float] = None | ||
| self.end_time: Optional[float] = None | ||
|
|
||
| def start_benchmark(self) -> None: | ||
| """ | ||
| Mark the start of the benchmark. Resets any prior state in the listener | ||
| by re-registering it to ensure clean collection. | ||
| """ | ||
| self._reset_listener_state() | ||
| self.start_time = time.time() | ||
|
|
||
| listener = None | ||
| def _reset_listener_state(self) -> None: | ||
| """ | ||
| Reset listener state by unregistering and re-registering. | ||
| This ensures no carryover from previous runs. | ||
| """ | ||
| try: | ||
| import python_listener | ||
| listener = python_listener.PythonListener() | ||
| listener.register() | ||
| except Exception as e: | ||
| print("Not found com.nvidia.spark.rapids.listener.Manager", str(e)) | ||
| listener = None | ||
| return listener | ||
| self.listener.unregister_spark_listener() | ||
| except Exception: | ||
| # Ignore if unregister fails (e.g., not registered) | ||
| pass | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| self.listener.register_spark_listener() | ||
|
|
||
| def _get_spark_conf(self): | ||
| if self._is_spark_400_or_later(): | ||
| from pyspark.sql import is_remote | ||
| if is_remote(): | ||
| get_all = getattr(self.spark_session.conf, 'getAll', None) | ||
| return get_all() if callable(get_all) else (get_all or []) | ||
| def end_benchmark(self, benchmark_name: str) -> None: | ||
| """ | ||
| Mark the end of the benchmark and trigger report generation. | ||
|
|
||
| try: | ||
| return self.spark_session.sparkContext._conf.getAll() | ||
| except Exception: | ||
| get_all = getattr(self.spark_session.conf, 'getAll', None) | ||
| return get_all() if callable(get_all) else (get_all or []) | ||
| Args: | ||
| benchmark_name: Name of the benchmark to include in the report. | ||
| """ | ||
| self.end_time = time.time() | ||
| self._collect_metrics(benchmark_name) | ||
| self._write_report(benchmark_name) | ||
|
|
||
| def _collect_metrics(self, benchmark_name: str) -> None: | ||
| """ | ||
| Collect all relevant metrics into benchmark_data. | ||
| Since PythonListener only exposes notify/register methods, | ||
| we assume it internally accumulates data and can be queried via notify. | ||
|
|
||
| def report_on(self, fn: Callable, warmup_iterations = 0, iterations = 1, *args): | ||
| """Record a function for its running environment, running status etc. and exclude sentive | ||
| information like tokens, secret and password Generate summary in dict format for it. | ||
| We simulate retrieval by triggering a final notification | ||
| with a 'collect' action to extract accumulated metrics. | ||
| """ | ||
| duration = self.end_time - self.start_time if self.start_time and self.end_time else 0.0 | ||
|
|
||
| Args: | ||
| fn (Callable): a function to be recorded | ||
| # Simulate metric extraction using the only available method: notify | ||
| task_failures_event = { | ||
| "action": "get_task_failures", | ||
| "timestamp": time.time() | ||
| } | ||
| task_failures = self.listener.notify(task_failures_event) | ||
|
|
||
| Returns: | ||
| dict: summary of the fn | ||
| final_plan_event = { | ||
| "action": "get_final_execution_plan", | ||
| "timestamp": time.time() | ||
| } | ||
| final_plan = self.listener.notify(final_plan_event) | ||
|
|
||
| # Aggregate benchmark data | ||
| self.benchmark_data = { | ||
| "benchmark": benchmark_name, | ||
| "start_time": self.start_time, | ||
| "end_time": self.end_time, | ||
| "duration_seconds": duration, | ||
| "task_failures": task_failures or [], | ||
| "final_execution_plan": final_plan or {}, | ||
| "metadata": { | ||
| "report_generated_at": time.time(), | ||
| "listener_type": type(self.listener).__name__ | ||
| } | ||
| } | ||
|
|
||
| def _write_report(self, benchmark_name: str) -> None: | ||
| """ | ||
| spark_conf = dict(self._get_spark_conf()) | ||
| env_vars = dict(os.environ) | ||
| redacted = ["TOKEN", "SECRET", "PASSWORD"] | ||
| filtered_env_vars = dict((k, env_vars[k]) for k in env_vars.keys() if not (k in redacted)) | ||
| self.summary['env']['envVars'] = filtered_env_vars | ||
| self.summary['env']['sparkConf'] = spark_conf | ||
| self.summary['env']['sparkVersion'] = self.spark_session.version | ||
| listener = self._register_python_listener() | ||
| if listener is not None: | ||
| print("TaskFailureListener is registered.") | ||
| Write the collected benchmark data to a JSON file in the output directory. | ||
|
|
||
| Args: | ||
| benchmark_name: Name of the benchmark used for filename. | ||
| """ | ||
| if not os.path.exists(self.output_dir): | ||
| os.makedirs(self.output_dir, exist_ok=True) | ||
|
|
||
| safe_name = "".join(c for c in benchmark_name if c.isalnum() or c in ('-', '_')).rstrip() | ||
| filename = os.path.join(self.output_dir, f"{safe_name}_benchmark_report.json") | ||
|
|
||
| try: | ||
| # warmup | ||
| for i in range(0, warmup_iterations): | ||
| fn(*args) | ||
| with open(filename, 'w', encoding='utf-8') as f: | ||
| json.dump(self.benchmark_data, f, indent=2, default=str) | ||
| except Exception as e: | ||
| print('ERROR WHILE WARMUP BEGIN') | ||
| print(e) | ||
| traceback.print_tb(e.__traceback__) | ||
| print('ERROR WHILE WARMUP END') | ||
|
|
||
| start_time = int(time.time() * 1000) | ||
| self.summary['startTime'] = start_time | ||
| # run the query | ||
| for i in range(0, iterations): | ||
| try: | ||
| start_time = int(time.time() * 1000) | ||
| fn(*args) | ||
| end_time = int(time.time() * 1000) | ||
| if listener and len(listener.failures) != 0: | ||
| self.summary['queryStatus'].append("CompletedWithTaskFailures") | ||
| else: | ||
| self.summary['queryStatus'].append("Completed") | ||
| except Exception as e: | ||
| # print the exception to ease debugging | ||
| print('ERROR BEGIN') | ||
| print(e) | ||
| traceback.print_tb(e.__traceback__) | ||
| print('ERROR END') | ||
| end_time = int(time.time() * 1000) | ||
| self.summary['queryStatus'].append("Failed") | ||
| self.summary['exceptions'].append(str(e)) | ||
| finally: | ||
| self.summary['queryTimes'].append(end_time - start_time) | ||
| if listener is not None: | ||
| listener.unregister() | ||
| return self.summary | ||
|
|
||
| def write_summary(self, prefix=""): | ||
| """_summary_ | ||
| # Log error to stderr since we can't raise in reporting path | ||
| error_msg = f"Failed to write benchmark report to {filename}: {str(e)}\n{traceback.format_exc()}" | ||
| print(error_msg, file=os.sys.stderr) | ||
|
|
||
| Args: | ||
| query_name (str): name of the query | ||
| prefix (str, optional): prefix for the output json summary file. Defaults to "". | ||
| def get_report_data(self) -> Dict[str, Any]: | ||
| """ | ||
| # Power BI side is retrieving some information from the summary file name, so keep this file | ||
| # name format for pipeline compatibility | ||
| filename = prefix + '-' + self.summary['query'] + '-' +str(self.summary['startTime']) + '.json' | ||
| self.summary['filename'] = filename | ||
| with open(filename, "w") as f: | ||
| json.dump(self.summary, f, indent=2) | ||
|
|
||
| def is_success(self): | ||
| """Check if the query succeeded, queryStatus == Completed | ||
| Retrieve the current benchmark data dictionary. | ||
|
|
||
| Returns: | ||
| A dictionary containing all collected metrics. | ||
| """ | ||
| return self.summary['queryStatus'][0] == 'Completed' | ||
| return dict(self.benchmark_data) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
//is not valid Python syntax — immediateSyntaxError// File: nds/PysparkBenchReport.pyon line 1 uses C++/JavaScript-style comments, which Python does not recognise. Python will raiseSyntaxError: invalid syntaxthe moment any code attempts toimportthis module, making the entire file completely unusable. The same problem exists inutils/python_benchmark_reporter/PysparkBenchReport.py(line 1) andutils/spark_utils.py(line 1).