-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add memory cache #12
Changes from 13 commits
2509c10
f5e85f1
d7bcc38
9f7e9a5
6d4111f
aed7fb8
4293251
2b1b890
090c72d
09c6735
b67a81d
9ebb871
d24bd5e
ee7aed2
b78f7df
2529358
6ee979a
4e71df2
3a1a538
e30d8bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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? | ||
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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
This file was deleted.
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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...