Skip to content

Commit 452e85b

Browse files
[data] fix: tolerate shared filesystem mkdir races (#3888)
Signed-off-by: jbernloehr <jbernloehr@nvidia.com> Co-authored-by: Kamran Jafari <kjafarisadeg@nvidia.com>
1 parent 4d40dfd commit 452e85b

4 files changed

Lines changed: 78 additions & 3 deletions

File tree

‎src/megatron/bridge/data/builders/finetuning_dataset.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,11 @@ def default_pack_path(self) -> Path:
339339
self.dataset_root / "packed" / f"{tokenizer_model_name}_pad_seq_to_mult{self._pad_seq_to_mult}"
340340
)
341341
if not default_pack_path.exists():
342-
default_pack_path.mkdir(parents=True, exist_ok=True)
342+
try:
343+
# Shared filesystems can expose stale parent-dir state despite exist_ok=True.
344+
default_pack_path.mkdir(parents=True, exist_ok=True)
345+
except (FileExistsError, FileNotFoundError):
346+
pass
343347
logger.info(f"Using default path for packing files: {str(default_pack_path)}")
344348

345349
return default_pack_path

‎src/megatron/bridge/data/datasets/sft.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,11 @@ def get_dataset_root(name: str) -> Path:
9191
Path: The path to the dataset's root directory.
9292
"""
9393
output = Path(NEMO_DATASETS_CACHE) / name
94-
output.mkdir(parents=True, exist_ok=True)
94+
try:
95+
# Shared filesystems can expose stale parent-dir state despite exist_ok=True.
96+
output.mkdir(parents=True, exist_ok=True)
97+
except (FileExistsError, FileNotFoundError):
98+
pass
9599

96100
return output
97101

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# Copyright (c) 2026, NVIDIA CORPORATION. All rights reserved.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
from pathlib import Path
16+
from unittest.mock import MagicMock
17+
18+
import pytest
19+
20+
from megatron.bridge.data.builders.finetuning_dataset import FinetuningDatasetBuilder
21+
from megatron.bridge.data.datasets.packed_sequence import PackedSequenceSpecs
22+
23+
24+
@pytest.mark.parametrize("mkdir_error", [FileExistsError, FileNotFoundError])
25+
def test_default_pack_path_ignores_shared_fs_mkdir_race(tmp_path, monkeypatch, mkdir_error):
26+
"""Network filesystems can leak mkdir races even with exist_ok=True."""
27+
builder = FinetuningDatasetBuilder(
28+
dataset_root=tmp_path,
29+
tokenizer=MagicMock(),
30+
packed_sequence_specs=PackedSequenceSpecs(
31+
packed_sequence_size=128,
32+
tokenizer_model_name="mock-tokenizer",
33+
pad_seq_to_mult=8,
34+
),
35+
)
36+
expected_path = tmp_path / "packed" / "mock-tokenizer_pad_seq_to_mult8"
37+
38+
monkeypatch.setattr(Path, "exists", lambda _: False)
39+
40+
def raise_mkdir(self, parents=False, exist_ok=False):
41+
assert self == expected_path
42+
assert parents is True
43+
assert exist_ok is True
44+
raise mkdir_error("stale shared filesystem state")
45+
46+
monkeypatch.setattr(Path, "mkdir", raise_mkdir)
47+
48+
assert builder.default_pack_path == expected_path

‎tests/unit_tests/data/datasets/test_sft.py‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,14 @@
1313
# limitations under the License.
1414

1515
import json
16+
from pathlib import Path
1617
from unittest.mock import MagicMock
1718

1819
import numpy as np
20+
import pytest
1921

20-
from megatron.bridge.data.datasets.sft import GPTSFTChatDataset, GPTSFTDataset, GPTSFTPackedDataset
22+
from megatron.bridge.data.datasets import sft as sft_module
23+
from megatron.bridge.data.datasets.sft import GPTSFTChatDataset, GPTSFTDataset, GPTSFTPackedDataset, get_dataset_root
2124

2225

2326
def create_mock_tokenizer():
@@ -98,6 +101,22 @@ def get_gpt_sft(tmp_path, dataset_type="sft"):
98101
return dataset, num_samples
99102

100103

104+
@pytest.mark.parametrize("mkdir_error", [FileExistsError, FileNotFoundError])
105+
def test_get_dataset_root_ignores_shared_fs_mkdir_race(tmp_path, monkeypatch, mkdir_error):
106+
"""Network filesystems can leak mkdir races even with exist_ok=True."""
107+
monkeypatch.setattr(sft_module, "NEMO_DATASETS_CACHE", tmp_path)
108+
109+
def raise_mkdir(self, parents=False, exist_ok=False):
110+
assert self == tmp_path / "sft"
111+
assert parents is True
112+
assert exist_ok is True
113+
raise mkdir_error("stale shared filesystem state")
114+
115+
monkeypatch.setattr(Path, "mkdir", raise_mkdir)
116+
117+
assert get_dataset_root("sft") == tmp_path / "sft"
118+
119+
101120
class TestDataGPTSFTDataset:
102121
def test_build_samples_mapping(self, tmp_path):
103122
dataset, _ = get_gpt_sft(tmp_path)

0 commit comments

Comments
 (0)