Caching store: keep the ranges read of large objects, and their sizes 30d5fac7560d
A large pack (over an eighth of the cache) was read by range, never kept, and every read asked the remote its size first: two requests per object. A server opens the store again for every request, so nothing learnt carried over: on oxygenhub.io a negotiate on metalgear2 (in GCS) made about 270 requests and took 7-8 s. Ranges read are now kept on disk under cached-range/KEY/START-END, and sizes under cached-size/KEY, counted against the limit and evicted like any object, and removed with their object; objects never change, so a kept range is always right. A test reads a range again through a store opened afresh on a remote that answers nothing. Refs andymccall/oxygenhub#234, andymccall/oxygenhub#228
- Author
- Andy McCall @andymccall <[email protected]>
- via
- agent
Claude Code <[email protected]> · delegate for changes, until 2026-11-13 19:00Z · key ea5f78160088
- When
- · 2026-10-17 00:51 UTC
- Signature
- ✔ signed by Claude Code
- Hash
30d5fac7560dea15f032e006ddd6d00a693ff81a8b1eb33875be65836fee74c2- Depends on
- 454236d1108d a5f76c79666c
- Depended on by
- nothing yet
- In bookmarks
- carried-not-removed doctor-trust identical-sides keyless-tags main merge-remote-bookmark paths-for-resolutions-and-amend push-notes show-stat subrepo-pin-state
- In tags
- v0.10.0 v0.11.0 v0.4.4 v0.4.5 v0.4.6 v0.5.0 v0.5.1 v0.6.0 v0.7.0 v0.8.0 v0.9.0
- Edits
- 10 atoms, 0 tree operations, 1 file
Browse the tree at this change
modified crates/oxy-core/src/strata.rs +99 −4
| … | 666 unchanged lines |
||
| 667 | 667 | used: Mutex<Option<u64>>, |
|
| 668 | 668 | } |
|
| 669 | 669 | ||
| 670 | + | /// Where a caching store keeps the ranges it read of objects too large to |
|
| 671 | + | /// keep whole, and those objects' sizes: apart from every object key. |
|
| 672 | + | const RANGES: &str = "cached-range"; |
|
| 673 | + | const SIZES: &str = "cached-size"; |
|
| 674 | + | ||
| 670 | 675 | /// How stale a cached object's use time may get before a read renews it. |
|
| 671 | 676 | const TOUCH_AFTER: Duration = Duration::from_secs(60); |
|
| 672 | 677 | ||
| … | 72 unchanged lines |
||
| 745 | 750 | } |
|
| 746 | 751 | *self.used.lock().unwrap_or_else(|p| p.into_inner()) = Some(left); |
|
| 747 | 752 | Ok(()) |
|
| 753 | + | } |
|
| 754 | + | ||
| 755 | + | /// An object's size: kept on disk once the remote has said it (objects |
|
| 756 | + | /// never change), so it is asked once. `None` when there is no object. |
|
| 757 | + | fn size_of(&self, key: &str) -> Result<Option<u64>> { |
|
| 758 | + | let skey = format!("{SIZES}/{key}"); |
|
| 759 | + | if let Some(n) = self |
|
| 760 | + | .cache |
|
| 761 | + | .get(&skey) |
|
| 762 | + | .ok() |
|
| 763 | + | .and_then(|b| String::from_utf8(b).ok()) |
|
| 764 | + | .and_then(|t| t.parse().ok()) |
|
| 765 | + | { |
|
| 766 | + | return Ok(Some(n)); |
|
| 767 | + | } |
|
| 768 | + | let size = self.remote.stat(key)?.map(|e| e.size); |
|
| 769 | + | if let Some(n) = size { |
|
| 770 | + | self.keep(&skey, n.to_string().as_bytes())?; |
|
| 771 | + | } |
|
| 772 | + | Ok(size) |
|
| 748 | 773 | } |
|
| 749 | 774 | ||
| 750 | 775 | /// Mark a cached object used now, at most once a minute. |
|
| … | 52 unchanged lines |
||
| 803 | 828 | Err(e) if self.cache.has(key)? => return Err(e), |
|
| 804 | 829 | Err(_) => {} |
|
| 805 | 830 | } |
|
| 831 | + | // A large object's ranges are kept as they are read (packs never |
|
| 832 | + | // change, oxygenhub#234): a server opens the store again for every |
|
| 833 | + | // request, so only what is on disk carries over. |
|
| 834 | + | let rkey = format!("{RANGES}/{key}/{}-{}", range.start, range.end); |
|
| 835 | + | if let Ok(bytes) = self.cache.get(&rkey) { |
|
| 836 | + | self.touch(&rkey); |
|
| 837 | + | return Ok(bytes); |
|
| 838 | + | } |
|
| 806 | 839 | // A small object is fetched whole and kept; a large one is read by |
|
| 807 | - | // range, every time. |
|
| 808 | - | match self.remote.stat(key)? { |
|
| 809 | - | Some(e) if self.cacheable(e.size) => { |
|
| 840 | + | // range and the range kept. Its size is kept too, so the remote is |
|
| 841 | + | // asked it once. |
|
| 842 | + | match self.size_of(key)? { |
|
| 843 | + | Some(size) if self.cacheable(size) => { |
|
| 810 | 844 | let bytes = self.get(key)?; |
|
| 811 | 845 | let len = bytes.len() as u64; |
|
| 812 | 846 | if range.end > len || range.start > range.end { |
|
| … | 3 unchanged lines |
||
| 816 | 850 | ))); |
|
| 817 | 851 | } |
|
| 818 | 852 | Ok(bytes[range.start as usize..range.end as usize].to_vec()) |
|
| 853 | + | } |
|
| 854 | + | Some(_) => { |
|
| 855 | + | let bytes = self.remote.get_range(key, range)?; |
|
| 856 | + | self.keep(&rkey, &bytes)?; |
|
| 857 | + | Ok(bytes) |
|
| 819 | 858 | } |
|
| 820 | - | Some(_) => self.remote.get_range(key, range), |
|
| 821 | 859 | None => Err(Error::NotFound(key.to_string())), |
|
| 822 | 860 | } |
|
| 823 | 861 | } |
|
| … | 35 unchanged lines |
||
| 859 | 897 | ||
| 860 | 898 | fn delete(&self, key: &str) -> Result<()> { |
|
| 861 | 899 | self.remote.delete(key)?; |
|
| 900 | + | // Its kept ranges and size go with it. |
|
| 901 | + | let mut extra = self.cache.list(&format!("{RANGES}/{key}/"))?; |
|
| 902 | + | extra.extend(self.cache.stat(&format!("{SIZES}/{key}"))?); |
|
| 903 | + | for e in extra { |
|
| 904 | + | self.cache.delete(&e.key)?; |
|
| 905 | + | let mut used = self.used.lock().unwrap_or_else(|p| p.into_inner()); |
|
| 906 | + | if let Some(u) = used.as_mut() { |
|
| 907 | + | *u = u.saturating_sub(e.size); |
|
| 908 | + | } |
|
| 909 | + | } |
|
| 862 | 910 | let size = self.cache.stat(key)?.map(|e| e.size); |
|
| 863 | 911 | self.cache.delete(key)?; |
|
| 864 | 912 | if let Some(n) = size { |
|
| … | 216 unchanged lines |
||
| 1081 | 1129 | assert!(!store.cache.has("refs").unwrap()); |
|
| 1082 | 1130 | } |
|
| 1083 | 1131 | ||
| 1132 | + | /// A remote that answers nothing: what is read through it was cached. |
|
| 1133 | + | struct Unreachable; |
|
| 1134 | + | ||
| 1135 | + | impl Store for Unreachable { |
|
| 1136 | + | fn describe(&self) -> String { |
|
| 1137 | + | "unreachable".into() |
|
| 1138 | + | } |
|
| 1139 | + | fn has(&self, _: &str) -> Result<bool> { |
|
| 1140 | + | Err(Error::Backend("asked the remote".into())) |
|
| 1141 | + | } |
|
| 1142 | + | fn get(&self, _: &str) -> Result<Vec<u8>> { |
|
| 1143 | + | Err(Error::Backend("asked the remote".into())) |
|
| 1144 | + | } |
|
| 1145 | + | fn get_range(&self, _: &str, _: Range<u64>) -> Result<Vec<u8>> { |
|
| 1146 | + | Err(Error::Backend("asked the remote".into())) |
|
| 1147 | + | } |
|
| 1148 | + | fn put_new(&self, _: &str, _: &[u8]) -> Result<()> { |
|
| 1149 | + | Err(Error::Backend("asked the remote".into())) |
|
| 1150 | + | } |
|
| 1151 | + | fn list(&self, _: &str) -> Result<Vec<Entry>> { |
|
| 1152 | + | Err(Error::Backend("asked the remote".into())) |
|
| 1153 | + | } |
|
| 1154 | + | fn delete(&self, _: &str) -> Result<()> { |
|
| 1155 | + | Err(Error::Backend("asked the remote".into())) |
|
| 1156 | + | } |
|
| 1157 | + | fn stat(&self, _: &str) -> Result<Option<Entry>> { |
|
| 1158 | + | Err(Error::Backend("asked the remote".into())) |
|
| 1159 | + | } |
|
| 1160 | + | fn read_ref(&self, _: &str) -> Result<Option<(Vec<u8>, Version)>> { |
|
| 1161 | + | Err(Error::Backend("asked the remote".into())) |
|
| 1162 | + | } |
|
| 1163 | + | fn write_ref(&self, _: &str, _: &[u8], _: Option<&Version>) -> Result<Version> { |
|
| 1164 | + | Err(Error::Backend("asked the remote".into())) |
|
| 1165 | + | } |
|
| 1166 | + | } |
|
| 1167 | + | ||
| 1084 | 1168 | #[test] |
|
| 1085 | 1169 | fn a_caching_store_stays_within_its_limit() { |
|
| 1086 | 1170 | let dir = tempfile::tempdir().unwrap(); |
|
| … | 27 unchanged lines |
||
| 1114 | 1198 | assert_eq!(store.get_range("pack-big", 10..20).unwrap(), vec![7u8; 10]); |
|
| 1115 | 1199 | assert_eq!(store.get("pack-big").unwrap().len(), 2000); |
|
| 1116 | 1200 | assert!(!store.cache.has("pack-big").unwrap()); |
|
| 1201 | + | // Its ranges are kept as read, and its size, so a store opened |
|
| 1202 | + | // again (a server's next request) asks the remote nothing. |
|
| 1203 | + | let again = |
|
| 1204 | + | CachingStore::open(Arc::new(Unreachable), dir.path().join("cache"), 8000).unwrap(); |
|
| 1205 | + | assert_eq!(again.get_range("pack-big", 10..20).unwrap(), vec![7u8; 10]); |
|
| 1117 | 1206 | // Counting from scratch agrees with what was kept track of. |
|
| 1118 | 1207 | let fresh = CachingStore::open(remote, dir.path().join("cache"), 8000).unwrap(); |
|
| 1119 | 1208 | assert_eq!(fresh.cached_bytes().unwrap(), store.cached_bytes().unwrap()); |
|
| 1209 | + | // Deleting it takes its ranges and size too. |
|
| 1210 | + | store.delete("pack-big").unwrap(); |
|
| 1211 | + | assert!( |
|
| 1212 | + | store.cache.list("cached-range/").unwrap().is_empty() |
|
| 1213 | + | && store.cache.list("cached-size/").unwrap().is_empty() |
|
| 1214 | + | ); |
|
| 1120 | 1215 | } |
|
| 1121 | 1216 | ||
| 1122 | 1217 | #[test] |
|
| … | 104 unchanged lines |