Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
20 changes: 12 additions & 8 deletions docs/source/prefetcher.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
GCSFS Adaptive Concurrent Prefetching: Architecture & Usage Guide
=================================================================

Prefetcher is not enabled by default. To enable, you need to pass the environment variable
`USE_EXPERIMENTAL_ADAPTIVE_PREFETCHING='true'` and `DEFAULT_GCSFS_CONCURRENCY`=4. As currently written, this implementation is
Prefetcher is enabled by default with `DEFAULT_GCSFS_CONCURRENCY=4` and `cache_type="none"`. To disable, you can pass the environment variable `USE_EXPERIMENTAL_ADAPTIVE_PREFETCHING='false'` or pass `use_experimental_adaptive_prefetching=False` when opening a file. As currently written, this implementation is
separate from the fsspec-style caching layer, but the intent is to eventually make this available to all
asynchronous filesystems using the standard `cache_type=` argument. How it interacts with the
existing cache types ("readahead", "first", etc.) remains to be decided, and in the meantime, use at your own risk.
Expand Down Expand Up @@ -71,17 +70,22 @@ Interaction with GCSFile

The prefetcher is integrated into the ``GCSFile`` and replaces the standard sequential fetching mechanism when enabled.

Enabling the Feature
--------------------
Feature Configuration & Disabling
---------------------------------

To use this architecture, set the following environment variables:
Adaptive prefetching is enabled by default with ``DEFAULT_GCSFS_CONCURRENCY=4`` and ``cache_type="none"``.

To disable prefetching and revert to the legacy readahead cache, set the environment variable:

.. code-block:: bash

export DEFAULT_GCSFS_CONCURRENCY=4
export USE_EXPERIMENTAL_ADAPTIVE_PREFETCHING='true'
export USE_EXPERIMENTAL_ADAPTIVE_PREFETCHING='false'

or pass ``use_experimental_adaptive_prefetching=False`` directly when opening a file:

.. code-block:: python

We recommend setting ``cache_type="none"`` for optimal results. The engine avoids prefetching for random workloads, and other cache types create unnecessary memory copies that degrade performance.
gcs.open("bucket/file.txt", "rb", use_experimental_adaptive_prefetching=False)

Under the Hood Lifecycle
------------------------
Expand Down
34 changes: 19 additions & 15 deletions gcsfs/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -2313,7 +2313,7 @@ def __init__(
mode="rb",
block_size=DEFAULT_BLOCK_SIZE,
autocommit=True,
cache_type="readahead",
cache_type=None,
cache_options=None,
acl=None,
consistency="md5",
Expand Down Expand Up @@ -2376,6 +2376,24 @@ def __init__(
raise OSError("Attempt to open a bucket")
self.generation = _coalesce_generation(generation, path_generation)
self.concurrency = kwargs.get("concurrency", DEFAULT_CONCURRENCY)
# Ideally, all of these fields should be part of `cache_options`. Because current
# `fsspec` caches do not accept arbitrary `*args` and `**kwargs`, passing them
# there currently causes instantiation errors. We are holding off on introducing
# them as explicit keyword arguments to ensure existing user workloads are not
# disrupted. This will be refactored once the upstream `fsspec` changes are merged.
if "use_experimental_adaptive_prefetching" in kwargs:
use_prefetch_reader = bool(kwargs["use_experimental_adaptive_prefetching"])
Comment thread
ankitaluthra1 marked this conversation as resolved.
Outdated
else:
use_prefetch_reader = os.environ.get(
"USE_EXPERIMENTAL_ADAPTIVE_PREFETCHING", "true"
).lower() in (
"true",
"1",
)
Comment on lines +2384 to +2395

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

There is a discrepancy in how use_experimental_adaptive_prefetching is resolved when explicitly passed as None in kwargs. In ZonalFile.__init__, an explicit None falls back to the environment variable default. However, in GCSFile.__init__, checking "use_experimental_adaptive_prefetching" in kwargs evaluates to True even if the value is None, which then evaluates bool(None) to False. This disables the prefetcher while keeping cache_type="none", leading to no caching or prefetching at all. Using kwargs.get() to handle None consistently resolves this issue.

Suggested change
if "use_experimental_adaptive_prefetching" in kwargs:
val = kwargs["use_experimental_adaptive_prefetching"]
use_prefetch_reader = (
val.lower() in ("true", "1") if isinstance(val, str) else bool(val)
)
else:
use_prefetch_reader = os.environ.get(
"USE_EXPERIMENTAL_ADAPTIVE_PREFETCHING", "true"
).lower() in (
"true",
"1",
)
val = kwargs.get("use_experimental_adaptive_prefetching")
if val is not None:
use_prefetch_reader = (
val.lower() in ("true", "1") if isinstance(val, str) else bool(val)
)
else:
use_prefetch_reader = os.environ.get(
"USE_EXPERIMENTAL_ADAPTIVE_PREFETCHING", "true"
).lower() in (
"true",
"1",
)


if cache_type is None:
cache_type = "none" if use_prefetch_reader else "readahead"
Comment thread
ankitaluthra1 marked this conversation as resolved.

super().__init__(
gcsfs,
path,
Expand All @@ -2394,20 +2412,6 @@ def __init__(
self.consistency = consistency
self.checker = get_consistency_checker(consistency)

# Ideally, all of these fields should be part of `cache_options`. Because current
# `fsspec` caches do not accept arbitrary `*args` and `**kwargs`, passing them
# there currently causes instantiation errors. We are holding off on introducing
# them as explicit keyword arguments to ensure existing user workloads are not
# disrupted. This will be refactored once the upstream `fsspec` changes are merged.
use_prefetch_reader = kwargs.get(
"use_experimental_adaptive_prefetching", False
) or os.environ.get(
"USE_EXPERIMENTAL_ADAPTIVE_PREFETCHING", "false"
).lower() in (
"true",
"1",
)

if "r" in mode and use_prefetch_reader:
max_prefetch_size = kwargs.get("max_prefetch_size", MAX_PREFETCH_SIZE)
from .prefetcher import BackgroundPrefetcher
Expand Down
16 changes: 13 additions & 3 deletions gcsfs/tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1598,7 +1598,7 @@ def test_errors(gcs):

def test_read_small(gcs):
fn = TEST_BUCKET + "/2014-01-01.csv"
with gcs.open(fn, "rb", block_size=10) as f:
with gcs.open(fn, "rb", block_size=10, cache_type="readahead") as f:
out = []
while True:
data = f.read(3)
Expand Down Expand Up @@ -1725,7 +1725,7 @@ def test_readline_from_cache(gcs):
with gcs.open(a, "wb") as f:
f.write(data)

with gcs.open(a, "rb") as f:
with gcs.open(a, "rb", cache_type="readahead") as f:
result = f.readline()
assert result == b"a,b\n"
assert f.loc == 4
Expand Down Expand Up @@ -2814,12 +2814,22 @@ async def mock_fail_seq(path, start, end, **kwargs):


def test_gcsfile_prefetch_disabled_fallback(gcs):
"""Verify that omitting the flag entirely skips the prefetcher initialization."""
"""Verify that disabling prefetcher defaults to readahead cache unless cache_type="none" is explicitly specified."""
fn = f"{TEST_BUCKET}/no_prefetch.txt"
gcs.pipe(fn, b"HelloWorld")

# When prefetcher disabled and no cache_type specified, defaults to readahead
with gcs.open(fn, "rb", use_experimental_adaptive_prefetching=False) as f:
assert getattr(f, "_prefetch_engine", None) is None
assert f.cache_type == "readahead"
assert f.read() == b"HelloWorld"

# When prefetcher disabled and cache_type="none" explicitly specified, remains "none"
with gcs.open(
fn, "rb", cache_type="none", use_experimental_adaptive_prefetching=False
) as f:
assert getattr(f, "_prefetch_engine", None) is None
assert f.cache_type == "none"
assert f.read() == b"HelloWorld"


Expand Down
6 changes: 3 additions & 3 deletions gcsfs/tests/test_zonal_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ def fake_sync(loop, func, *args, **kwargs):
assert result == [b"split_data"]
mock_gcsfs._fetch_range_split.assert_awaited_once_with(
zf.path,
concurrency=1,
concurrency=4,
start=10,
chunk_lengths=[5],
size=zf.size,
Expand Down Expand Up @@ -711,7 +711,7 @@ def test_zonal_file_pool_size_initialization(mock_sync, mock_gcsfs):
mode="rb",
use_experimental_adaptive_prefetching=True,
)
assert zf2.pool_size == 1
assert zf2.pool_size == 4
assert zf2._prefetch_engine is not None
zf2.close()

Expand All @@ -721,7 +721,7 @@ def test_zonal_file_pool_size_initialization(mock_sync, mock_gcsfs):
mode="rb",
use_experimental_adaptive_prefetching=False,
)
assert zf3.pool_size == 1
assert zf3.pool_size == 4
assert zf3._prefetch_engine is None
zf3.close()

Expand Down
2 changes: 1 addition & 1 deletion gcsfs/zb_hns_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
)

MRD_MAX_RANGES = 1000 # MRD supports up to 1000 ranges per request
DEFAULT_CONCURRENCY = int(os.environ.get("DEFAULT_GCSFS_CONCURRENCY", "1"))
DEFAULT_CONCURRENCY = int(os.environ.get("DEFAULT_GCSFS_CONCURRENCY", "4"))
Comment thread
ankitaluthra1 marked this conversation as resolved.
Outdated
MAX_PREFETCH_SIZE = 256 * 1024 * 1024
logger = logging.getLogger("gcsfs")

Expand Down
Loading