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

RFC: tf.data Snapshot #193

Merged
merged 11 commits into from
Feb 11, 2020
368 changes: 368 additions & 0 deletions rfcs/20200107-tf-data-snapshot.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,368 @@
# tf.data Snapshot

| Status | Proposed |
| :------------ | :------------------------------------------------------ |
| **RFC #** | [193](https://github.com/tensorflow/community/pull/193) |
| **Author(s)** | Frank Chen ([email protected]), Rohan Jain |
| | ([email protected]) |
| **Sponsor** | Jiri Simsa ([email protected]) |
| **Updated** | 2020-01-07 |

## Objective

With ever faster accelerators available in Cloud and hyperparameter tuning
consuming larger chunks of accelerator time, TensorFlow users are increasingly
finding that they don’t have enough CPU resources to keep up with these
accelerators, leaving valuable accelerator resources idle.

To alleviate this problem, we are proposing a `snapshot` API within `tf.data`,
to allow users to transparently persist the output of their preprocessing
pipeline to disk, and materialize the pre-processed data on a different training
run.

This API enables repeated preprocessing steps to be consolidated, and allowing
re-use of already processed data, trading off disk storage and network bandwidth
for freeing up more valuable CPU resources and accelerator compute time.

## Motivation

Large TensorFlow users have indicated that they have complicated input
processing pipelines which saturate their CPUs before saturating their
accelerators (TPUs in particular). Since they often experiment with
hyperparameter tuning or tweaks to existing model without affecting their input
pipeline, they are asking for ways to avoid similar repeated preprocessing of
data by either saving a dataset or caching it to disk.

## User Benefit

Users will be able to transparently persist partially or fully processed data
from `tf.data` input pipelines to disk or Cloud storage systems, and materialize
the pre-processed data during subsequent runs from the same pipeline. This will
cut down on the input pipeline processing overheads during second and subsequent
runs.

## Design Proposal

We propose that we add a new `snapshot` transformation to tf.data. To illustrate
the usage of the transformation, we can start with some sample code:

```python
dataset = Dataset.list_files("/raw/data/*").shard(num_workers, i)
dataset = dataset.parallel_interleave(TFRecordDataset)
dataset = dataset.map(my_preprocessing_fn)
dataset = dataset.apply(tf.data.snapshot("/saved/data", options...))
Copy link
Contributor

Choose a reason for hiding this comment

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

either dataset = dataset.apply(tf.data.experimental.snapshot(...)) or dataset = dataset.snapshot(...) (I would prefer the former before providing backwards compatibility guarantee for the API).

Copy link

Choose a reason for hiding this comment

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

Why apply --> snapshot? Is there ever a case where you would snapshot without the .apply call? If not, can we include the apply under the hood, and let the users just dataset.snapshot(...)?

Copy link
Member

Choose a reason for hiding this comment

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

I think right now .apply(...) is the only way to support an experimental tf.data transformation. So the plan would be to expose this as apply and then in a few months, we can promote this to a dataset.snapshot API.

dataset = dataset.repeat()

model = ...
model.fit(dataset)
```

As we can see, the end user simply has to add this transformation in order to
use this functionality. In essence, the transformation is similar to the
existing `tf.data.Dataset.cache`, with the key difference is being that, unlike
`cache`, `snapshot` is intended to re-used across different executions of the
Copy link
Contributor

Choose a reason for hiding this comment

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

Does cache provide any advantages over snapshot?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cache can be in memory, which is useful for testing and small datasets.

Copy link
Contributor

@byronyi byronyi Jan 11, 2020

Choose a reason for hiding this comment

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

Cache can be on-disk as well. Its lifetime is more confined (to a specific job), and no worries about crash-safe semantics. Lifetime of snapshots could be unbounded, and I guess that's why we need to explicitly expire them.

same input pipelines.

### Proposed API

We are proposing the following API for the snapshot transformation.

```python
def snapshot(path,
compression=None,
reader_path_prefix=None,
writer_path_prefix=None,
shard_size_bytes=None,
pending_snapshot_expiry_seconds=None,
num_reader_threads=None,
reader_buffer_size=None,
num_writer_threads=None,
writer_buffer_size=None,
shuffle_on_read=None,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does snapshotting include its own shuffling?

How does the snapshot shuffling interact with the shuffling performed in existing tf.data transformations like shuffle?

The way these options are presented now mean you have different behavior when loading from a snapshot than you had when not loading from a snapshot, which means the existing shuffling behavior is not guaranteed even if there is no shuffle_on_read (as reading from a snapshot cannot simulate shuffling files).

shuffle_seed=None,
mode=None,
snapshot_name=None):
Copy link

Choose a reason for hiding this comment

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

This is a lot of args, especially to start off with. Is this perhaps a sign that we should be thinking of a different design pattern here?

Copy link
Member

Choose a reason for hiding this comment

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

Agreed! How about a SnapshotConfig / Options object? Or did you have something else in mind?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I don't particularly like how many args there are here either. Perhaps reader/writer threads and buffer sizes can be collapsed into just threads and buffer (cc @rohan100jain)

Copy link

Choose a reason for hiding this comment

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

Is a config object better? Or just another level of indirection? I feel like users are fatigued by all our TF config objects. One option is to start with fewer, and see which knobs users actually ask for-- because this is experimental, we can easily add. Are there any of these that can be made subsequent API calls? Eg, a set_options or set_buffer_size? I obviously don't have a good solution here-- only problems ;)

Copy link
Member

Choose a reason for hiding this comment

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

Would the AUTO as default help here? I think based on our experience, we can come up with reasonable defaults reducing the amount of cognitive overhead the users will have to endure figuring out what to set.

We can also remove the buffer_size option and just set it to 2 * num_threads.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After discussion, going to remove the buffer_size option.

Copy link
Contributor

@byronyi byronyi Jan 11, 2020

Choose a reason for hiding this comment

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

Would the AUTO as default help here? I think based on our experience, we can come up with reasonable defaults reducing the amount of cognitive overhead the users will have to endure figuring out what to set.

We can also remove the buffer_size option and just set it to 2 * num_threads.

Would you mind to leave it there? Buffer size is an important parameter to tune for distributed FS such as S3 or HDFS. The appropriate buffer size should be bandwidth delay product, not 2*num_threads. This option will be relevant if we would like to maximise the read/write throughput to the underlying storage with both high delay and high available bandwidth.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The buffer size configuration here is for the reading and writing thread buffers (basically the same effect as having Dataset.prefetch(1)) rather than file system buffers. For the buffers you are talking about, we offer configurations at the file system levels (https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/platform/cloud/gcs_file_system.cc#L67) that would be more general.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the clarification. I am just wondering where can I pass system buffer configuration with the proposed snapshot API (and in general APIs that interact with external storage such as cache).

pass # Implementation goes here.
```

1. `path`: Required. A directory where we want to save our snapshots and/or
read from a previously saved snapshot.

2. `compression`: Optional. The type of compression to apply to the snapshot
written to disk. This will support `GZIP`, `SNAPPY` or None. Defaults to
None.
Copy link
Member

Choose a reason for hiding this comment

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

Should it default to AUTO? Note that suffixes can be added to the produced files to make it clear to the user (and tools) what the encoding is as well.

Eg .gz

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. We can have the default option be AUTO here and elsewhere, and gradually add tf.data autotuning to snapshots after (autotuning right now starts a few versions of the pipelines, which is not ideal given that we are writing to the file system).


3. `reader_path_prefix`: Optional. A prefix to add to the path when reading
from snapshots. This is useful for filesystems where configuration is passed
in through the path. Defaults to None.

4. `writer_path_prefix`: Optional. A prefix to add to the path when writing to
snapshots. This is useful for filesystems where configuration is passed in
through the path. Defaults to None.

5. `shard_size_bytes`: Optional. The maximum size of each data file to be
written by the snapshot dataset op. Defaults to 10 GiB.

6. `pending_snapshot_expiry_seconds`: Optional. How long to wait (in seconds)
before the snapshot op considers a previously unfinished snapshot to be
stale and starts writing a snapshot from scratch again. Defaults to 86400
seconds (1 day).

7. `num_reader_threads`: Optional. Number of threads to parallelize reading
from snapshot. Especially useful if compression is turned on since the
decompression operation tends to be intensive. Defaults to 1. If > 1, then
this might introduce non-determinism i.e. the order in which the elements
are read from the snapshot are different from the order they're written.

8. `reader_buffer_size`: Optional. Maximum number of elements we can prefetch
reading from the snapshot. Defaults to 1. Increasing this might improve
Copy link
Member

Choose a reason for hiding this comment

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

Should it default to AUTO? Also, not sure what "1" means here.

Similarly for writter_buffer_size below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed

performance but will increase memory consumption.

9. `num_writer_threads`: Optional. Number of threads to parallelize writing
from snapshot. We'll open up `num_writer_threads` files and write to them in
parallel. Especially useful if compression is turned on since the
compression operation tends to be intensive. Defaults to 1. If > 1, then
this might introduce non-determinism i.e. the order in which the elements
are read from the upstream iterator are different from the order they're
written.

10. `writer_buffer_size`: Optional. Maximum number of pipeline elements to fill
up the buffer before writing them out using `num_writer_threads`.

11. `shuffle_on_read`: Optional. If this is True, then the order in which
examples are produced when reading from a snapshot will be random. Defaults
Copy link
Member

Choose a reason for hiding this comment

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

random or arbitrary? Is this a local shuffle, a global shuffle, something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clarified. The behavior emulates Dataset.list_files(shuffle=True) by randomizing the order in which the snapshot files are read back.

to False.
Copy link

Choose a reason for hiding this comment

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

Why is this an arg versus a call to dataset.shuffle() after the call to tf.data.snapshot? Coupling this + the following kwarg into the API spec here seems unwieldy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This emulates the behavior when Dataset.list_files(shuffle=True) and is orthogonal to using dataset.shuffle() after the snapshot op. Clarified the behavior of this flag.


12. `shuffle_seed`: Optional. If shuffle_seed is set, the random number
generator used for shuffling (when `shuffle_on_read` is turned on) is seeded
by the given seed. Otherwise, it is seeded by a random seed that differs for
every run.

13. `mode`: Optional. The mode at which snapshot should operate. Valid options
are `auto`, `read`, `write`, and `passthrough`. The default mode is `auto`,
where the snapshot op will automatically determine what mode to operate in.

1. `write` mode forces the snapshot transformation to write a new
materialization to disk, regardless of whether a complete and valid
materialization currently exists. In other words, we enter the **WRITE**
state immediately.

2. `read` mode forces the snapshot transformation to read from the latest
version of the materialization on disk, regardless of whether the data
stored on disk is complete and valid. In other words, we enter the
**READ** state immediately.

3. `passthrough` mode turns the snapshot transformation into a no-op. In
other words, we enter the **PASSTHROUGH** state immediately.

4. `auto` retains the default behavior of snapshot. See the "Standard
Kernel Workflow" section for the default behavior.

14. `snapshot_name`: Optional. If set, use the supplied string as a named
snapshot name instead of introspecting the data pipeline and automatically
generating a unique identifier for the specific data pipeline.

1. Instead of generating a new fingerprint of the input processing graph or
and `run_id` (see the _Detailed Design_ section for details), we will
use the `snapshot_name` to uniquely identify the snapshot.

### External API Guarantees

Externally, we guarantee that snapshots written by a particular version of
TensorFlow will be readable by that specific version of TensorFlow. Eventually,
we can also guarantee that snapshots written will be readable by all future
Copy link

Choose a reason for hiding this comment

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

all future-- we don't guarantee that for SavedModels, and all feels strong for any API. Version N+1 is what we guarantee for SavedModel, and that may be as strong as you want to go, realistically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. Removed the guarantees about future stuff.

versions of TensorFlow.

We are not currently handling the case where workers do not go through the
entire training set at least once.

### Alternatives Considered

An alternative proposal for an API would be `save()` and `load()`, where the
saving and loading of the input pipeline would be made more explicit, avoiding
some of the logic needed in determining whether to snapshot or read from a
snapshot of a model.

The downside here would be that the user would have to split the preprocessing
and training into potentially different files, and users would be forced to
select whether to train or preprocess on their own, which is not good.

Copy link

Choose a reason for hiding this comment

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

I feel like save/load fits better into existing TF paradigms, and makes the behavior more explicit. Looking at the pipeline above, it's hard to tell what is going to be run on an arbitrary run of the pipeline. You could imagine an individual pipeline doing a load first, then if no file to load, do the preproc/save. That's explicit and doesn't require two separate runners.

Copy link
Member

Choose a reason for hiding this comment

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

The issue with that is the logic that determines whether the input pipeline we want to load or not is not as simple as whether a file exists (the control aspect of the op). Following your train of thought, I think the better alternative would be splitting it up as reader, writer and control as separate ops (which as I indicated above should be the implementation for the next iteration of the op).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this design was chosen so that the user doesn't have to worry about changing their existing pipelines much at all, and can benefit from this just by dropping snapshot in.

With save() and load(), users will have to manage saving and loading by themselves and this potentially introduces errors (e.g. users may want accidentally load an old version of a snapshot without realizing it).

That said, perhaps we can re-use the C++ code that implements this into more generic Dataset.save and Dataset.load ops for users who want that sort of control?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree with @rohan100jain that for the next internal implementation we can expose save/load/control where appropriate while retaining this python interface for existing users who might want a simple solution.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should consider that the semantics of save/load are much easier to explain to people specially when you consider the interaction of shuffle_on_read with other options earlier in the input pipeline.

Copy link
Member

Choose a reason for hiding this comment

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

During earlier iterations of developing this, we considered save / load style APIs but to make that usable is quite challenging (i.e. the figuring out whether to save a new snapshot or to load). The current API serves a very concrete use case (for some significant internal users) and we feel it makes sense to expose this API to serve that use case.

Your concerns about shuffle_on_read etc. are valid and we'll address them by allowing users to specify a reader_fn.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alextp @rohan100jain I have updated the design doc to add a new reader_fn and remove existing reader parameters. PTAL, thanks!

### Performance Implications

* Do you expect any (speed / memory)? How will you confirm?
* There should be microbenchmarks. Are there?
* There should be end-to-end tests and benchmarks. If there are not (since
this is still a design), how will you track that these will be created?

### Dependencies

No new dependencies will be introduced as part of this project to TensorFlow.
Dependent projects may be able to use this additional op, but there should be no
significant changes otherwise.

### Engineering Impact

Binary sizes increases slightly with the inclusion of this new op, and this code
will be maintained by the `tf.data` team.

### Platforms and Environments

This op will work on all TensorFlow-supported platforms. We do not anticipate
this to work on embedded systems as it is not useful in resource-constrained
environments.

### Best Practices, Tutorials and Examples

A user guide for snapshot will be published to guide new users in using this
feature.

### Compatibility

This introduces a new op, which will impact future backwards compatibility.

### User Impact

A new python function and a new op are the only user-facing changes visible.

## Detailed Design

### Implementation Assumptions

The following implementation is based on the following assumptions that define
the MVP this is designed for:

1. We assume that at least for one pipeline run, you can go through the entire
training dataset and be able to store that data on disk. Otherwise, a
snapshot will never get created.

2. In case there are multiple workers and the dataset is sharded across
workers, we assume that the number of workers remains the same from one run
to another. If the number changes, we’ll trigger another snapshot.
Copy link
Member

Choose a reason for hiding this comment

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

I guess this also affects seed semantics?

Copy link
Contributor

@byronyi byronyi Jan 9, 2020

Choose a reason for hiding this comment

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

I would suggest reconsidering this. Elastic training reduces resource fragmentation and improve resource utilization in general. Sometimes the number of workers, and along with learning rate, global batch size, etc. are important hyperparameters to tune by themselves.

What about a collective snapshot design in the context of a distributed file system? Will this be enough for an easier implementation without this assumption?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@byronyi The current tf.data input pipeline design doesn't really allow us to coordinate between workers (e.g. there might failure cases where K worker out of N fails and the snapshot slice is invalid).

That said, I totally understand where you are coming from, and we are planning a data pipeline service (where snapshot can be integrated) so everyone would read elements off a single pipeline and having different numbers of trainers would be less of an issue. There should be a design doc RFC on that in the next few weeks so look for that.

Copy link

Choose a reason for hiding this comment

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

How will we trigger another snapshot exactly? Does that imply attempting a read of the data, failing, and instead re-snapshotting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the number of workers change, then (at least in the case of sharded TPU training) the num_shards parameter to Dataset.shard will change too. This results in a different graph fingerprint for each worker, and we will go into WRITE mode automatically. If there are no shards and all the workers use exactly the same input pipeline, then this won't happen. Will add something here to clarify.


3. Any `repeat`s in the dataset should be moved to after the `snapshot` op, to
avoid writing large (or infinite) amounts of data during a snapshot writing
run.

### New `SnapshotDatasetOp`

To implement the transformation, we are introducing a new `SnapshotDatasetOp`
dataset kernel that will implement all of the functionality in TensorFlow C++.
Python code is mostly glue code to pass relevant parameters into the op kernel.

### Internal Directory / File Structure

Given a user directory path (e.g. `/path/to/snapshot`), the directory will look
like:

* /path/to/snapshot
* `fingerprint`/
* snapshot.metadata
* `run-id`/
* 0000000.snapshot
* 0000001.snapshot

The `fingerprint` is a hash of the input processing graph. The `run-id` is
unique training run ID generated.
Copy link

Choose a reason for hiding this comment

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

How does this align/not align with existing ckpt + saved model semantics? Is there more we can reuse here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think SavedModel supports comparisons between graphs (or in this case, parts of a graph), which is our main use case here. In general, I am not sure TensorFlow can guarantee to produce the exact same graph (i.e. with the same node names, function names, etc...), so we are utilizing the HashGraph function (https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/kernels/data/dataset_utils.cc#L733), which computes a fingerprint while ignoring node names and other things that may vary run by run.

Copy link

Choose a reason for hiding this comment

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

That is true, though I meant the file structure itself and the serialized format, not strictly the fingerprint.

Copy link
Member

Choose a reason for hiding this comment

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

Thats a good point Karmel and we considered this while first developing. Here is why we ended up choosing something of our own -

The checkpointing API is for saving the state of the model and here we're trying to store the entire output of an input pipeline and so API wise there isn't much alignment.

On the implementation side, Checkpointing uses BundleReader / BundleWriter which are designed for key / value accesses to tensors whereas here we want to sequentially read off tensors as soon as possible. We tried Bundle Reader / Writer at first but abandoned it for performance reasons.

Copy link

Choose a reason for hiding this comment

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

At the risk of sacrilege-- can we improve the performance of Bundle Reader / Writer instead of just building a new way of doing this?

Copy link
Contributor

@byronyi byronyi Jan 16, 2020

Choose a reason for hiding this comment

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

Update: with a little bit tweak I managed to get the performance of tensor bundle from 4.5k image/s to 7.2k image/s using 128 threads. Writes are scattered into multiple shards so it wouldn't be too hard to get maximal performance. Up till now the only constrtaint seems to be my 10GbE NIC, and I plan to do another benchmark on a machine with 25GbE network.

Copy link
Member

Choose a reason for hiding this comment

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

@karmel While I think its a worthy objective of unifying the Bundle Reader / Writer file format with what we're proposing, I don't want to tie them both together this early. The reason is that since the workloads are quite different, we are still early days into exactly knowing what sort of options and functionality we'll have to build in to get the maximum throughput (e.g. we've needed compression so that we don't saturate network bandwidth too quickly, different threading implementations etc.). So my proposal would be to keep it separate at the moment, gain some experience tuning this workload and then we can pull in some of the learnings into Bundle Reader / Writer.

Copy link
Contributor

@byronyi byronyi Jan 21, 2020

Choose a reason for hiding this comment

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

@rohan100jain I completely agree with you. At the end of day we will be all better off with a format that yields highest performance possible, and we will definitely learned how to build one when experimenting a new format built from scratch.

Being said that, I would also like to keep improving tensor bundle, as the current on-disk cache uses it as underlying storage. @karmel would you be interested in sponsoring an RFC regarding to that? It could serve as a primary input source (TFIndexedRecord), accelerating on-disk CacheDataset, and enhance ShuffleDataset with external shuffle. We could focus on the API first. If Rohan and @frankchn designed a more performant format it could be used for these APIs as well.

Copy link

Choose a reason for hiding this comment

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

Thanks, @byronyi -- @rohan100jain / @frankchn , what was the conclusion from the design review as to input format + readers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@karmel -- We are going to re-investigate TFRecordReader performance and see if we can bring that up to equivalency with our implementation.


### Standard Kernel Workflow

_Note: This is an implementation detail, and may change in the future. This
should not be relied upon except as a reference to the current implementation._

By default, the `snapshot` operation will, upon startup, make a determination
using the following algorithm as to whether the operation should be in the
WRITE, PASSTHROUGH, or READ state.

1. We will compute a graph fingerprint containing all the information from the
Dataset preprocessing graph before the `snapshot` op. We’ll use the
`AsGraphDefInternal` method on DatasetBase for this.

1. We will attempt to enter the corresponding fingerprint directory. For
instance, if the computed fingerprint is `f-abc123` and the base snapshot
directory is `/saved/data`, then we will attempt to enter
`/saved/data/f-abc123`.

1. If the snapshot directory is non-existent, empty or it doesn’t contain a
`metadata` file, we will enter the **WRITE** state.

1. If the snapshot directory contains a `metadata` file, we will read the
metadata file.

1. The metadata file contains the following fields:
Copy link
Member

Choose a reason for hiding this comment

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

Does this suggest that metadata file needs to be modifiable after initial write? Any concurrency implications? Does it make sense to have an approach that works with immutable File artifacts?

Some enhancements that were recently made to TensorFlow Transform might be of interest here (especially their logic and rational behind them, if not the implementation).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An immutable metadata file definitely makes sense. We will introduce a metadata.final file to indicate when snapshot has finished writing.

1. A training run ID
1. A boolean indicating if the snapshot is complete
1. A training run start-time.

1. If the training run start-time is more than the (configurable) training run
timeout (set with the `pending_snapshot_expiry_seconds` parameter), we will
enter the **WRITE** state.

1. If the training run start-time is less than the training run timeout, but
the snapshot is not complete, then we will enter the **PASSTHROUGH** state.

1. If the snapshot is complete, we will enter the **READ** state.

#### WRITE State

1. We generate a random training run ID.

1. We write (possibly overwriting) the `snapshot.metadata` file.

1. We proceed to create a subdirectory containing the training run ID, and
start writing data asynchronously in chunks.

1. At the end of the dataset (when `end_of_sequence == true`), we will check
the snapshot.metadata file to determine whether it contains the same
training run ID.

1. If it does, we set the complete bit to true to finalize the directory.
1. If it does not, it means that someone else is concurrently writing the
snapshot and we lost the race to them. We delete all data in the
training run directory.

For the current implementation, we will store the data in chunked TFRecord
files. Eventually we may move to other more higher performance data stores or
Copy link
Member

Choose a reason for hiding this comment

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

Do you envision this a "sink" configuration in the API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@katsiapis Sorry, can you clarify what sink you are referring to?

Copy link
Member

Choose a reason for hiding this comment

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

I guess Gus is referring to allowing users to customize the writing data format etc. via some kind of sink configuration. I think as of now, we don't plan to allow for that level of customization because right now its good to have our own reader / writer that is optimized for performance. Its definitely possible that in the future we allow users to customize this though but I think that won't happen in the near future. Will keep this in mind though.

support additional storage systems such as Cloud BigTable.

#### PASSTHROUGH State

1. This is a no-op, where we simply pass through the tensors to the downstream
operations.

#### READ State

1. We will read from the snapshots contained within the subfolder with the
correct graph fingerprint and specified training run ID.

1. Optionally, the user may choose to tell us to specify that the snapshots
should be read back in shuffled order.

### Concurrency: Handling Multiple Input Workers

If input workers are sharded, then they will generate different graph
fingerprints as their shard indexes will be different. This will result in each
worker writing to a different subdirectory.

If input workers are not sharded, then this will result in a race and
potentially multiple workers writing data (still with different training run
IDs). Eventually, if each worker finishes, we will be left with one copy of the
data as all the other workers will determine that they have lost the race and
delete their own copy of the snapshot data.

## Questions and Discussion Topics

* Should we implement this as three ops (a control opt o determine whether a
snapshot is to be read from/written to) and a write and read op to do the
respective operations?
* Pros include:
* Modularizes the implementation into smaller chunks
* Allows someone else to do the "control"
* Challenges include:
* Where/how the "control" runs?
* How do we construct the dataset graph properly?
* How should autotuning be integrated into the snapshot transformation?
* Are the configuration options well named? Is it possible to consolidate some
of these options?
* What other compression/decompression options would you like to see
supported?
* Any other performance / feature tuning knobs we should make available?