Skip to content
This repository has been archived by the owner on Jun 6, 2024. It is now read-only.

feat: add memory cache #12

Merged
merged 20 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
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
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
run: cargo clippy --all-targets --all-features
- name: Compile check
run: cargo check --all-targets --all-features
- name: Generate code coverage
- name: Run tests & Generate code coverage
run: cargo llvm-cov --all-features --workspace --codecov --output-path lcov.info
- name: Archive code coverage results
uses: actions/upload-artifact@v4
Expand Down
32 changes: 26 additions & 6 deletions storage-node/TODO.md
Original file line number Diff line number Diff line change
@@ -1,15 +1,35 @@
### Features
1. Add in-memory cache for small and hot data.
For in-memory cache, currently we can have 2 designs of the whole procedure for write data into cache:
Generally, memory cache will record a hashmap (key -> vector of Bytes) to record the data. (Note: in current version, the file represents all the files for one S3 request, and we should optimize it later.)
1. First Design:
1. Get the file size and file from S3.
2. Storage_manager will check the file size, if it is small, try to put it into memory cache, (remember also write the evicted memory key to disk if applicable); otherwise, try to write it into disk cache.

(Disk cache and memory cache could consume a `S3Reader` as an input parameter for write_data in this case)
2. Second Dsign:
1. Get the file from S3 (as S3Reader).
2. Storage_manager will convert S3Reader to stream, and call next to get `Bytes`. Then, it will first try to write it to memory cache.

Memory cache will first add the key into its hashmap, and record the `Bytes`. But when it finds the size is too large, it will return false and also return the vector of `Bytes` for this key.

Storage_manager will write the file with `false` return value from memory cache `or` the evicted file to disk.

(Disk cache and memory cache should accept `Bytes` as an input parameter for write_data in this case)

For me, the first design is very clear, and the second design can save the cost of requesting S3 to get the size. But I think the cost is very low. Maybe the major difference is the API for write_data?
Copy link
Member

Choose a reason for hiding this comment

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

If we're going to send an additional HTTP request for every S3 request to get the file size, then the cost may not be neglectable. But I do vote for the first design cuz it's more straightforward and less prone to bugs...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Due to API constraints, I currently adopt method 2.
As expected, the code becomes very ugly : ( Let me think about how to refine it...

2. Integrate RocksDB.
3. Implement client and its communicaiton with the server.
4. Pin & unpin to prevent eviction the disk file that is using. (Although this situation seems to rarely happen)
### Optimization
1. Discuss the key for the cache. Currently it is raw S3 request string, but maybe we can make it as `bucket + one key`, since if one key represents a file, then for different requests, keys may overlap, and the current implementation will waste disk space.
2. Solve the read-write & write-write conflict for disk operations. Let's say we have 2 requests, and they have same key for the cache, the first request has a cache miss, then it has to go to S3 to fetch data. But now second request comes, and the first request hasn't pulled all the data from S3 and stored them in the disk. So the disk file exists, but the content is not complete. Here is one possible way: when a cache miss happens, we record this key into our cache structure, but with a status `uncompleted`, and when the data is fully pulled and stored, the status is updated to `completed`. When another same request comes and tries to `get` disk file from cache, our cache finds the key exists but its status is `uncompleted`. So the cache should wait until its status turns to `completed`. (The async design to make the cache not simply dry wait is worth discussing)
3. In async approach, we should support read & output data at the same time, both S3 read & disk write(or network write) and disk read & network write. To support this, we cannot use one single fixed buffer, which is the current implementation of disk_manager. Here is 2 ways to support this:
1. For cache policy like `lru.rs` and `lru_k.rs`, we can use fine-grained locks instead of one big lock to lock it. If we use fine-grained locks (at least after getting the value from cache, we don't need to hold its lock), we should add unpin/pin in `lru.rs` and `lru_k.rs`.
2. Discuss the key for the cache. Currently it is raw S3 request string, but maybe we can make it as `bucket + one key`, since if one key represents a file, then for different requests, keys may overlap, and the current implementation will waste disk space.
Copy link
Member

Choose a reason for hiding this comment

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

Will raw S3 request string be different for the same file? Isn't it something like aws s3 cp s3://bucket-name/path/to/file.txt /local/path/file.txt? In other words, if keys overlap, won't the requests also overlap? (Not sure, maybe wrong)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I left this comment based on the current s3.rs test, so I am not sure what is the raw S3 request string... But if it the same as the s3.rs test (one S3 request string can be converted to one bucket + many keys, and each key represents one file), we should consider the cache key more carefully in the future. I added this todo just to remind us double check the format of raw S3 request string.
Feel free to correct me, since I almost know nothing about S3 requests. I think it depends on one raw S3 request string containing only one file or multiple files.

Copy link
Member

Choose a reason for hiding this comment

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

Will raw S3 request string be different for the same file?

Yes. When we make a request to an S3 object, we must specify its bucket AND its key. So we need separate requests if a table spans multiple keys in a bucket.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let me correct myself. The raw S3 request string means the input param from storage manager.
So a quick question: for the raw S3 request string in storage manager, can it be transformed to one bucket and multiple keys?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we have to talk to the catalog team to finalize the data scheme

3. Solve the read-write & write-write conflict for disk operations. Let's say we have 2 requests, and they have same key for the cache, the first request has a cache miss, then it has to go to S3 to fetch data. But now second request comes, and the first request hasn't pulled all the data from S3 and stored them in the disk. So the disk file exists, but the content is not complete. Here is one possible way: when a cache miss happens, we record this key into our cache structure, but with a status `uncompleted`, and when the data is fully pulled and stored, the status is updated to `completed`. When another same request comes and tries to `get` disk file from cache, our cache finds the key exists but its status is `uncompleted`. So the cache should wait until its status turns to `completed`. (The async design to make the cache not simply dry wait is worth discussing)
4. In async approach, we should support read & output data at the same time, both S3 read & disk write(or network write) and disk read & network write. To support this, we cannot use one single fixed buffer, which is the current implementation of disk_manager. Here is 2 ways to support this:
1. a buffer consumed and extended(see `s3.rs`);
2. two fixed buffers.

(Apart from it, do we have another method to make async version gain more performance than `sync` with `multiple threads`?)
4. Discuss other possibilities to parallel.
5. Design benchmark and add statistics for benchmark.
6. For stream and iterator, buffer_size is one thing we should consider. If it is too small, then we have to do more frequent I/Os, but if it is too big, then we cannot serve too many requests at the same time due to fixed total memory size. Maybe we can adjust the buffer_size dynamically.
5. Discuss other possibilities to parallel.
6. Design benchmark and add statistics for benchmark.
7. For stream and iterator, buffer_size is one thing we should consider. If it is too small, then we have to do more frequent I/Os, but if it is too big, then we cannot serve too many requests at the same time due to fixed total memory size. Maybe we can adjust the buffer_size dynamically.
29 changes: 0 additions & 29 deletions storage-node/src/cache/data/mod.rs

This file was deleted.

Loading
Loading