Skip to content
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

to_zarr() is extremely slow writing to high latency store #277

Closed
slevang opened this issue Nov 15, 2023 · 5 comments
Closed

to_zarr() is extremely slow writing to high latency store #277

slevang opened this issue Nov 15, 2023 · 5 comments
Labels
IO Representation of particular file formats as trees

Comments

@slevang
Copy link
Contributor

slevang commented Nov 15, 2023

Unbearably so, I would say. Here is an example with a tree containing 13 nodes and negligible data, trying to write to S3/GCS with fsspec:

import numpy as np
import xarray as xr
from datatree import DataTree

ds = xr.Dataset(
    data_vars={
        "a": xr.DataArray(np.ones((2, 2)), coords={"x": [1, 2], "y": [1, 2]}),
        "b": xr.DataArray(np.ones((2, 2)), coords={"x": [1, 2], "y": [1, 2]}),
        "c": xr.DataArray(np.ones((2, 2)), coords={"x": [1, 2], "y": [1, 2]}),
    }
)

dt = DataTree()
for first_level in [1, 2, 3]:
    dt[f"{first_level}"] = DataTree(ds)
    for second_level in [1, 2, 3]:
        dt[f"{first_level}/{second_level}"] = DataTree(ds)

%time dt.to_zarr("test.zarr", mode="w")

bucket = "s3|gs://your-bucket/path" 
%time dt.to_zarr(f"{bucket}/test.zarr", mode="w")

Gives:

CPU times: user 53.8 ms, sys: 3.95 ms, total: 57.8 ms
Wall time: 58 ms

CPU times: user 6.33 s, sys: 211 ms, total: 6.54 s
Wall time: 3min 20s

I suspect one of the culprits may be that we're having to reopen the store without consolidated metadata on writing each node:

datatree/datatree/io.py

Lines 205 to 223 in 433f78d

for node in dt.subtree:
ds = node.ds
group_path = node.path
if ds is None:
_create_empty_zarr_group(store, group_path, mode)
else:
ds.to_zarr(
store,
group=group_path,
mode=mode,
encoding=encoding.get(node.path),
consolidated=False,
**kwargs,
)
if "w" in mode:
mode = "a"
if consolidated:
consolidate_metadata(store)

Any ideas for easy improvements here?

@jhamman
Copy link

jhamman commented Nov 15, 2023

Many many ideas for improvements. The Zarr backend we wrote was really meant to be an MVP, it absolutely needs some work. Here's my diagnosis:

  1. As mentioned, opening / listing each group independently is inefficient. This could be addressed here in Datatree.
  2. Xarray sequentially initializes each group and array, then updates the user attributes. Any batching here would help. This should probably be addressed upstream in Xarray and Zarr-Python.

My approach to (2) is to rethink the Zarr-Python API for creating hierarchies. You may be interested in the discussion here: zarr-developers/zarr-python#1569

@slevang
Copy link
Contributor Author

slevang commented Nov 15, 2023

Awesome, thanks for the info! I imagine (1) would require reimplementing a good chunk of xarray's ZarrStore and other backend objects here in a way that avoids as many of these serial ops as possible?

@TomNicholas TomNicholas added the IO Representation of particular file formats as trees label Nov 15, 2023
@slevang
Copy link
Contributor Author

slevang commented Nov 15, 2023

In the meantime, this is plenty fast for the small data case:

def to_zarr(dt, path):
    with TemporaryDirectory() as tmp_path:
        dt.to_zarr(tmp_path)
        fs.put(tmp_path, path, recursive=True)

Takes 1s on my example above instead of 3m.

@TomNicholas
Copy link
Collaborator

@slevang would you mind performing the same test with xarray.core.datatree.DataTree upstream? Then I will know whether or not this issue still exists even after many changes (in xarray, datatree, and zarr). If it still exists can you please re-raise it on the xarray main repo :)

@slevang
Copy link
Contributor Author

slevang commented Sep 8, 2024

Looks like things are better but still very slow. The example in the OP now takes just over a minute on latest versions writing to GCS. DataTree.to_zarr hasn't changed so the improvement must be higher up in the stack.

I've done a little profiling, and the fundamental problem is still that we're synchronously creating each group via a separate Dataset.to_zarr call. This involves a bunch of calls to various fsspec methods to check path existence and write small attribute and metadata files. The example above writes 13 groups so that adds up quickly.

To make this significantly better, unfortunately I think we need to drop the reliance on Dataset.to_zarr and rebuild the method to write the whole group structure in one go, and then write the data.

I'll do a little more digging and reopen on xarray.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IO Representation of particular file formats as trees
Projects
None yet
Development

No branches or pull requests

3 participants