Skip to content

Fix corrupted JSONL metrics file due to concurrent writes #19011

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

Merged
Fridge003 merged 4 commits into from
Feb 22, 2026
Merged

Fix corrupted JSONL metrics file due to concurrent writes #19011

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8c643a2
Fixed async writing to the same file in metrics exporter
talorabr Feb 19, 2026
39eec95
Changed to RWLock utility
talorabr Feb 19, 2026
4c21e27
Merge branch 'main' into bugfix/file_requests_metric_exporter_concurr…
talorabr Feb 19, 2026
1e35220
changed to regular asyncio lock
talorabr Feb 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions python/sglang/srt/managers/request_metrics_exporter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from sglang.srt.managers.io_struct import EmbeddingReqInput, GenerateReqInput
from sglang.srt.server_args import ServerArgs
from sglang.srt.utils.aio_rwlock import RWLock

@gemini-code-assist gemini-code-assist Bot Feb 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since only the writer lock is used in FileRequestMetricsExporter, a simpler asyncio.Lock would suffice and be slightly more direct. You can remove this import and use the asyncio module, which is already imported.

@Fridge003 Fridge003 Feb 22, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@talorabr Can we replace with a simpler asyncio.Lock as suggested


logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -87,6 +88,7 @@ def __init__(

# File handler state management
self._current_file_handler = None
self._current_file_lock = RWLock()

@gemini-code-assist gemini-code-assist Bot Feb 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using a standard asyncio.Lock is simpler here since there are no concurrent read operations. While RWLock is not incorrect, asyncio.Lock is more idiomatic for purely exclusive access. If you anticipate adding concurrent read-only operations in the future, keeping RWLock is reasonable.

Suggested change
self._current_file_lock = RWLock()
self._current_file_lock = asyncio.Lock()

self._current_hour_suffix = None

def _ensure_file_handler(self, hour_suffix: str):
Expand Down Expand Up @@ -135,20 +137,21 @@ async def write_record(
current_time = datetime.now()
hour_suffix = current_time.strftime("%Y%m%d_%H")

# Ensure correct file handler is open for current hour
self._ensure_file_handler(hour_suffix)
async with self._current_file_lock.writer_lock:

@gemini-code-assist gemini-code-assist Bot Feb 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

To align with the suggestion of using asyncio.Lock, this should be updated to acquire the lock directly.

Suggested change
async with self._current_file_lock.writer_lock:
async with self._current_file_lock:

# Ensure correct file handler is open for current hour
self._ensure_file_handler(hour_suffix)

if self._current_file_handler is None:
return
if self._current_file_handler is None:
return

metrics_data = self._format_output_data(obj, out_dict)
metrics_data = self._format_output_data(obj, out_dict)

def write_file():
json.dump(metrics_data, self._current_file_handler)
self._current_file_handler.write("\n")
self._current_file_handler.flush()
def write_file():
json.dump(metrics_data, self._current_file_handler)
self._current_file_handler.write("\n")
self._current_file_handler.flush()

await asyncio.to_thread(write_file)
await asyncio.to_thread(write_file)
except Exception as e:
logger.exception(f"Failed to write perf metrics to file: {e}")

Expand Down
Loading