{"push_id":2656142893,"size":4,"distinct_size":4,"ref":"refs/heads/next","head":"b6b564783679b96da3472b3165449e161dbdcb0e","before":"e55034a33ea74dd48d6f0cebd8a32a9668732413","commits":[{"sha":"b9d51b4c08f0088ed2c70cb3f26adcfa72cad797","author":{"name":"Botond Dénes","email":"a584fa4d75a0a85d721f44fb6e06315a2853b0b6@scylladb.com"},"message":"tests/querier_cache: fix memory based eviction test\n\nDo increment the key counter after inserting the first querier into the\ncache. Otherwise two queriers with the same key will be inserted and\nwill fail the test. This problem is exposed by the changes the next\npatches make to the querier-cache but will be fixed before to maintain\nbisectability of the code.\n\nFixes: #3529","distinct":true,"url":"https://api.github.com/repos/scylladb/scylla/commits/b9d51b4c08f0088ed2c70cb3f26adcfa72cad797"},{"sha":"7ce7f3f0cc0812276fc160869d9a8d3add5a2409","author":{"name":"Botond Dénes","email":"a584fa4d75a0a85d721f44fb6e06315a2853b0b6@scylladb.com"},"message":"querier_cache: restructure entries storage\n\nCurrently querier_cache uses a `std::unordered_map<utils::UUID, querier>`\nto store cache entries and an `std::list<meta_entry>` to store meta\ninformation about the querier entries, like insertion order, expiry\ntime, etc.\n\nAll cache eviction algorithms use the meta-entry list to evict entries\nin reverse insertion order (LRU order). To make this possible\nmeta-entries keep an iterator into the entry map so that given a\nmeta-entry one can easily erase the querier entry. This however poses a\nproblem as std::unordered_map can possibly invalidate all its iterators\nwhen new items are inserted. This is use-after-free waiting to happen.\n\nAnother disadvantages of the current solution is that it requires the\nmeta-entry to use a weak pointer to the querier entry so that in case\nthat is removed (as a result of a successful lookup) it doesn't try to\naccess it. This has an impact on all cache eviction algorithms as they\nhave to be prepared to deal with stale meta-entries. Stale meta-entries\nalso unnecesarily consume memory.\n\nTo solve these problems redesign how querier_cache stores entries\ncompletely. Instead of storing the entries in an `std::unordered_map`\nand storing the meta-entries in an `std::list`, store the entries in an\n`std::list` and an intrusive-map (index) for lookups. This new design\nhas severeal advantages over the old one:\n* The entries will now be in insert order, so eviction strategies can\n  work on the entry list itself, no need to involve additional data\n  structures for this.\n* All data related to an entry is stored in one place, no data\n  duplication.\n* Removing an entry automatically removes it from the index as intrusive\n  containers support auto unlink. This means there is no need to store\n  iterators for long terms, risking use-after-free when the container\n  invalidates it's iterators.\n\nAdditional changes:\n* Modify eviction strategies so that they work with the `entry`\n  interface rather than the stored value directly.\n\nRef #3424","distinct":true,"url":"https://api.github.com/repos/scylladb/scylla/commits/7ce7f3f0cc0812276fc160869d9a8d3add5a2409"},{"sha":"2609a17a23e8ea5290a50f1f0192c7c23e14d4f3","author":{"name":"Botond Dénes","email":"a584fa4d75a0a85d721f44fb6e06315a2853b0b6@scylladb.com"},"message":"querier: find_querier(): return end() when no querier matches the range\n\nWhen none of the queriers found for the lookup key match the lookup\nrange `_entries.end()` should be returned as the search failed. Instead\nthe iterator returned from the failed `std::find_if()` is returned\nwhich, if the find failed, will be the end iterator returned by the\nprevious call to `_entries.equal_range()`. This is incorrect because as\nlong as `equal_range()`'s end iterator is not also `_entries.end()` the\nsearch will always return an iterator to a querier regardless of whether\nany of them actually matches the read range.\nFix by returning `_entries.end()` when it is detected that no queriers\nmatch the range.\n\nFixes: #3530","distinct":true,"url":"https://api.github.com/repos/scylladb/scylla/commits/2609a17a23e8ea5290a50f1f0192c7c23e14d4f3"},{"sha":"b6b564783679b96da3472b3165449e161dbdcb0e","author":{"name":"Avi Kivity","email":"8f920f22884d6fea9df883843c4a8095a2e5ac6f@scylladb.com"},"message":"Merge \"Fix querier-cache related issues\" from Botond\n\n\"\nThis mini series fixes some querier-cache related issues discovered\nwhile working on stateful range-scans.\n1) A problem in the memory based cache eviction test that is is yet\n   unexposed (#3529).\n2) Possible usage of invalidated iterators in querier_cache (#3424).\n3) lookup() possibly returning a querier with the wrong read range\n   (#3530).\n\nTests: unit(release)\n\"\n\n* 'fix-querier-cache-invalid-iterators-master' of https://github.com/denesb/scylla:\n  querier: find_querier(): return end() when no querier matches the range\n  querier_cache: restructure entries storage\n  tests/querier_cache: fix memory based eviction test","distinct":true,"url":"https://api.github.com/repos/scylladb/scylla/commits/b6b564783679b96da3472b3165449e161dbdcb0e"}]}
{"actor":{"display_login":"avikivity"}}