小组成员:谢瑞阳、徐翔宇
You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.

407 rivejä
12 KiB

fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
fix problems in LevelDB's caching code Background: LevelDB uses a cache (util/cache.h, util/cache.cc) of (key,value) pairs for two purposes: - a cache of (table, file handle) pairs - a cache of blocks The cache places the (key,value) pairs in a reference-counted wrapper. When it returns a value, it returns a reference to this wrapper. When the client has finished using the reference and its enclosed (key,value), it calls Release() to decrement the reference count. Each (key,value) pair has an associated resource usage. The cache maintains the sum of the usages of the elements it holds, and removes values as needed to keep the sum below a capacity threshold. It maintains an LRU list so that it will remove the least-recently used elements first. The max_open_files option to LevelDB sets the size of the cache of (table, file handle) pairs. The option is not used in any other way. The observed behaviour: If LevelDB at any time used more file handles concurrently than the cache size set via max_open_files, it attempted to reduce the number by evicting entries from the table cache. This could happen most easily during compaction, and if max_open_files was low. Because the handles were in use, their reference count did not drop to zero, and so the usage sum in the cache was not modified by the evictions. Subsequent Insert() calls returned valid handles, but their entries were immediately evicted from the cache, which though empty still acted as though full. As a result, there was effectively no caching, and the number of open file handles rose []ly until it hit system-imposed limits and the process died. If one set max_open_files lower, the cache was more likely to exhibit this beahviour, and cause the process to run out of file descriptors. That is, max_open_files acted in almost exactly the opposite manner from what was intended. The problems: 1. The cache kept all elements on its LRU list eligible for capacity eviction---even those with outstanding references from clients. This was ineffective in reducing resource consumption because there was an outstanding reference, guaranteeing that the items remained. A secondary issue was that there is no guarantee that these in-use items will be the last things reached in the LRU chain, which actually recorded "least-recently requested" rather than "least-recently used". 2. The sum of usages was decremented not when a (key,value) was evicted from the cache, but when its reference count went to zero. Thus, when things were removed from the cache, either by garbage collection or via Erase(), the usage sum was not necessarily decreased. This allowed the cache to act as though full when it was in fact not, reducing caching effectiveness, and leading to more resources being consumed---the opposite of what the evictions were intended to achieve. 3. (minor) The cache's clients insert items into it by first looking up the key, and inserting only if no value is found. Although the cache has an internal lock, the clients use no locking to ensure atomicity of the Lookup/Insert pair. (see table/table.cc: block_cache->Insert() and db/table_cache.cc: cache_->Insert()). Thus, if two threads Insert() at about the same time, they can both Lookup(), find nothing, and both Insert(). The second Insert() would evict the first value, leaving each thread with a handle on its own version of the data, and with the second version in the cache. It would be better if both threads ended up with a handle on the same (key,value) pair, which implies it must be the first item inserted. This suggests that Insert() should not replace an existing value. This can be made safe with current usage inside LeveDB itself, but this is not easy to change first because Cache is a public interface, so to change the semantics of an existing call might break things, second because Cache is an abstract virtual class, so adding a new abstract virtual method may break other implementations, and third, the new method "insert without replacing" cannot be implemented in terms of the existing methods, so cannot be implemented with a non-abstract default. But fortunately, the effects of this issue are minor, so this issue is not fixed by this change. The changes: The assumption in the fixes is that it is always better to cache entries unless removal from the cache would lead to deallocation. Cache entries now have an "in_cache" boolean indicating whether the cache has a reference on the entry. The only ways that this can become false without the entry being passed to its "deleter" are via Erase(), via Insert() when an element with a duplicate key is inserted, or on destruction of the cache. The cache now keeps two linked lists instead of one. All items in the cache are in one list or the other, and never both. Items still referenced by clients but erased from the cache are in neither list. The lists are: - in-use: contains the items currently referenced by clients, in no particular order. (This list is used for invariant checking. If we removed the check, elements that would otherwise be on this list could be left as disconnected singleton lists.) - LRU: contains the items not currently referenced by clients, in LRU order A new internal Ref() method increments the reference count. If incrementing from 1 to 2 for an item in the cache, it is moved from the LRU list to the in-use list. The Unref() call now moves things from the in-use list to the LRU list if the reference count falls to 1, and the item is in the cache. It no longer adjusts the usage sum. The usage sum now reflects only what is in the cache, rather than including still-referenced items that have been evicted. The LRU_Append() now takes a "list" parameter so that it can be used to append either to the LRU list or the in-use list. Lookup() is modified to use the new Ref() call, rather than adjusting the reference count and LRU chain directly. Insert() eviction code is also modified to adjust the usage sum and the in_cache boolean of the evicted elements. Some LevelDB tests assume that there will be no caching whatsoever if the cache size is set to zero, so this is handled as a special case. A new private method FinishErase() is factored out with the common code from where items are removed from the cache. Erase() is modified to adjust the usage sum and the in_cache boolean of the erased elements, and to use FinishErase(). Prune() is modified to use FinishErase() also, and to make use of the fact that the lru_ list now contains only items with reference count 1. - EvictionPolicy is modified to test that an entry with an outstanding handle is not evicted. This test fails with the old cache.cc. - A new test case UseExceedsCacheSize verifies that even when the cache is overfull of entries with outstanding handles, none are evicted. This test fails with the old cache.cc, and is the key issue that causes file descriptors to run out when the cache size is set too small. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123247237
8 vuotta sitten
  1. // Copyright (c) 2011 The LevelDB Authors. All rights reserved.
  2. // Use of this source code is governed by a BSD-style license that can be
  3. // found in the LICENSE file. See the AUTHORS file for names of contributors.
  4. #include <assert.h>
  5. #include <stdio.h>
  6. #include <stdlib.h>
  7. #include "leveldb/cache.h"
  8. #include "port/port.h"
  9. #include "port/thread_annotations.h"
  10. #include "util/hash.h"
  11. #include "util/mutexlock.h"
  12. namespace leveldb {
  13. Cache::~Cache() {
  14. }
  15. namespace {
  16. // LRU cache implementation
  17. //
  18. // Cache entries have an "in_cache" boolean indicating whether the cache has a
  19. // reference on the entry. The only ways that this can become false without the
  20. // entry being passed to its "deleter" are via Erase(), via Insert() when
  21. // an element with a duplicate key is inserted, or on destruction of the cache.
  22. //
  23. // The cache keeps two linked lists of items in the cache. All items in the
  24. // cache are in one list or the other, and never both. Items still referenced
  25. // by clients but erased from the cache are in neither list. The lists are:
  26. // - in-use: contains the items currently referenced by clients, in no
  27. // particular order. (This list is used for invariant checking. If we
  28. // removed the check, elements that would otherwise be on this list could be
  29. // left as disconnected singleton lists.)
  30. // - LRU: contains the items not currently referenced by clients, in LRU order
  31. // Elements are moved between these lists by the Ref() and Unref() methods,
  32. // when they detect an element in the cache acquiring or losing its only
  33. // external reference.
  34. // An entry is a variable length heap-allocated structure. Entries
  35. // are kept in a circular doubly linked list ordered by access time.
  36. struct LRUHandle {
  37. void* value;
  38. void (*deleter)(const Slice&, void* value);
  39. LRUHandle* next_hash;
  40. LRUHandle* next;
  41. LRUHandle* prev;
  42. size_t charge; // TODO(opt): Only allow uint32_t?
  43. size_t key_length;
  44. bool in_cache; // Whether entry is in the cache.
  45. uint32_t refs; // References, including cache reference, if present.
  46. uint32_t hash; // Hash of key(); used for fast sharding and comparisons
  47. char key_data[1]; // Beginning of key
  48. Slice key() const {
  49. // next_ is only equal to this if the LRU handle is the list head of an
  50. // empty list. List heads never have meaningful keys.
  51. assert(next != this);
  52. return Slice(key_data, key_length);
  53. }
  54. };
  55. // We provide our own simple hash table since it removes a whole bunch
  56. // of porting hacks and is also faster than some of the built-in hash
  57. // table implementations in some of the compiler/runtime combinations
  58. // we have tested. E.g., readrandom speeds up by ~5% over the g++
  59. // 4.4.3's builtin hashtable.
  60. class HandleTable {
  61. public:
  62. HandleTable() : length_(0), elems_(0), list_(nullptr) { Resize(); }
  63. ~HandleTable() { delete[] list_; }
  64. LRUHandle* Lookup(const Slice& key, uint32_t hash) {
  65. return *FindPointer(key, hash);
  66. }
  67. LRUHandle* Insert(LRUHandle* h) {
  68. LRUHandle** ptr = FindPointer(h->key(), h->hash);
  69. LRUHandle* old = *ptr;
  70. h->next_hash = (old == nullptr ? nullptr : old->next_hash);
  71. *ptr = h;
  72. if (old == nullptr) {
  73. ++elems_;
  74. if (elems_ > length_) {
  75. // Since each cache entry is fairly large, we aim for a small
  76. // average linked list length (<= 1).
  77. Resize();
  78. }
  79. }
  80. return old;
  81. }
  82. LRUHandle* Remove(const Slice& key, uint32_t hash) {
  83. LRUHandle** ptr = FindPointer(key, hash);
  84. LRUHandle* result = *ptr;
  85. if (result != nullptr) {
  86. *ptr = result->next_hash;
  87. --elems_;
  88. }
  89. return result;
  90. }
  91. private:
  92. // The table consists of an array of buckets where each bucket is
  93. // a linked list of cache entries that hash into the bucket.
  94. uint32_t length_;
  95. uint32_t elems_;
  96. LRUHandle** list_;
  97. // Return a pointer to slot that points to a cache entry that
  98. // matches key/hash. If there is no such cache entry, return a
  99. // pointer to the trailing slot in the corresponding linked list.
  100. LRUHandle** FindPointer(const Slice& key, uint32_t hash) {
  101. LRUHandle** ptr = &list_[hash & (length_ - 1)];
  102. while (*ptr != nullptr &&
  103. ((*ptr)->hash != hash || key != (*ptr)->key())) {
  104. ptr = &(*ptr)->next_hash;
  105. }
  106. return ptr;
  107. }
  108. void Resize() {
  109. uint32_t new_length = 4;
  110. while (new_length < elems_) {
  111. new_length *= 2;
  112. }
  113. LRUHandle** new_list = new LRUHandle*[new_length];
  114. memset(new_list, 0, sizeof(new_list[0]) * new_length);
  115. uint32_t count = 0;
  116. for (uint32_t i = 0; i < length_; i++) {
  117. LRUHandle* h = list_[i];
  118. while (h != nullptr) {
  119. LRUHandle* next = h->next_hash;
  120. uint32_t hash = h->hash;
  121. LRUHandle** ptr = &new_list[hash & (new_length - 1)];
  122. h->next_hash = *ptr;
  123. *ptr = h;
  124. h = next;
  125. count++;
  126. }
  127. }
  128. assert(elems_ == count);
  129. delete[] list_;
  130. list_ = new_list;
  131. length_ = new_length;
  132. }
  133. };
  134. // A single shard of sharded cache.
  135. class LRUCache {
  136. public:
  137. LRUCache();
  138. ~LRUCache();
  139. // Separate from constructor so caller can easily make an array of LRUCache
  140. void SetCapacity(size_t capacity) { capacity_ = capacity; }
  141. // Like Cache methods, but with an extra "hash" parameter.
  142. Cache::Handle* Insert(const Slice& key, uint32_t hash,
  143. void* value, size_t charge,
  144. void (*deleter)(const Slice& key, void* value));
  145. Cache::Handle* Lookup(const Slice& key, uint32_t hash);
  146. void Release(Cache::Handle* handle);
  147. void Erase(const Slice& key, uint32_t hash);
  148. void Prune();
  149. size_t TotalCharge() const {
  150. MutexLock l(&mutex_);
  151. return usage_;
  152. }
  153. private:
  154. void LRU_Remove(LRUHandle* e);
  155. void LRU_Append(LRUHandle*list, LRUHandle* e);
  156. void Ref(LRUHandle* e);
  157. void Unref(LRUHandle* e);
  158. bool FinishErase(LRUHandle* e) EXCLUSIVE_LOCKS_REQUIRED(mutex_);
  159. // Initialized before use.
  160. size_t capacity_;
  161. // mutex_ protects the following state.
  162. mutable port::Mutex mutex_;
  163. size_t usage_ GUARDED_BY(mutex_);
  164. // Dummy head of LRU list.
  165. // lru.prev is newest entry, lru.next is oldest entry.
  166. // Entries have refs==1 and in_cache==true.
  167. LRUHandle lru_ GUARDED_BY(mutex_);
  168. // Dummy head of in-use list.
  169. // Entries are in use by clients, and have refs >= 2 and in_cache==true.
  170. LRUHandle in_use_ GUARDED_BY(mutex_);
  171. HandleTable table_ GUARDED_BY(mutex_);
  172. };
  173. LRUCache::LRUCache()
  174. : usage_(0) {
  175. // Make empty circular linked lists.
  176. lru_.next = &lru_;
  177. lru_.prev = &lru_;
  178. in_use_.next = &in_use_;
  179. in_use_.prev = &in_use_;
  180. }
  181. LRUCache::~LRUCache() {
  182. assert(in_use_.next == &in_use_); // Error if caller has an unreleased handle
  183. for (LRUHandle* e = lru_.next; e != &lru_; ) {
  184. LRUHandle* next = e->next;
  185. assert(e->in_cache);
  186. e->in_cache = false;
  187. assert(e->refs == 1); // Invariant of lru_ list.
  188. Unref(e);
  189. e = next;
  190. }
  191. }
  192. void LRUCache::Ref(LRUHandle* e) {
  193. if (e->refs == 1 && e->in_cache) { // If on lru_ list, move to in_use_ list.
  194. LRU_Remove(e);
  195. LRU_Append(&in_use_, e);
  196. }
  197. e->refs++;
  198. }
  199. void LRUCache::Unref(LRUHandle* e) {
  200. assert(e->refs > 0);
  201. e->refs--;
  202. if (e->refs == 0) { // Deallocate.
  203. assert(!e->in_cache);
  204. (*e->deleter)(e->key(), e->value);
  205. free(e);
  206. } else if (e->in_cache && e->refs == 1) {
  207. // No longer in use; move to lru_ list.
  208. LRU_Remove(e);
  209. LRU_Append(&lru_, e);
  210. }
  211. }
  212. void LRUCache::LRU_Remove(LRUHandle* e) {
  213. e->next->prev = e->prev;
  214. e->prev->next = e->next;
  215. }
  216. void LRUCache::LRU_Append(LRUHandle* list, LRUHandle* e) {
  217. // Make "e" newest entry by inserting just before *list
  218. e->next = list;
  219. e->prev = list->prev;
  220. e->prev->next = e;
  221. e->next->prev = e;
  222. }
  223. Cache::Handle* LRUCache::Lookup(const Slice& key, uint32_t hash) {
  224. MutexLock l(&mutex_);
  225. LRUHandle* e = table_.Lookup(key, hash);
  226. if (e != nullptr) {
  227. Ref(e);
  228. }
  229. return reinterpret_cast<Cache::Handle*>(e);
  230. }
  231. void LRUCache::Release(Cache::Handle* handle) {
  232. MutexLock l(&mutex_);
  233. Unref(reinterpret_cast<LRUHandle*>(handle));
  234. }
  235. Cache::Handle* LRUCache::Insert(
  236. const Slice& key, uint32_t hash, void* value, size_t charge,
  237. void (*deleter)(const Slice& key, void* value)) {
  238. MutexLock l(&mutex_);
  239. LRUHandle* e = reinterpret_cast<LRUHandle*>(
  240. malloc(sizeof(LRUHandle)-1 + key.size()));
  241. e->value = value;
  242. e->deleter = deleter;
  243. e->charge = charge;
  244. e->key_length = key.size();
  245. e->hash = hash;
  246. e->in_cache = false;
  247. e->refs = 1; // for the returned handle.
  248. memcpy(e->key_data, key.data(), key.size());
  249. if (capacity_ > 0) {
  250. e->refs++; // for the cache's reference.
  251. e->in_cache = true;
  252. LRU_Append(&in_use_, e);
  253. usage_ += charge;
  254. FinishErase(table_.Insert(e));
  255. } else { // don't cache. (capacity_==0 is supported and turns off caching.)
  256. // next is read by key() in an assert, so it must be initialized
  257. e->next = nullptr;
  258. }
  259. while (usage_ > capacity_ && lru_.next != &lru_) {
  260. LRUHandle* old = lru_.next;
  261. assert(old->refs == 1);
  262. bool erased = FinishErase(table_.Remove(old->key(), old->hash));
  263. if (!erased) { // to avoid unused variable when compiled NDEBUG
  264. assert(erased);
  265. }
  266. }
  267. return reinterpret_cast<Cache::Handle*>(e);
  268. }
  269. // If e != nullptr, finish removing *e from the cache; it has already been
  270. // removed from the hash table. Return whether e != nullptr.
  271. bool LRUCache::FinishErase(LRUHandle* e) {
  272. if (e != nullptr) {
  273. assert(e->in_cache);
  274. LRU_Remove(e);
  275. e->in_cache = false;
  276. usage_ -= e->charge;
  277. Unref(e);
  278. }
  279. return e != nullptr;
  280. }
  281. void LRUCache::Erase(const Slice& key, uint32_t hash) {
  282. MutexLock l(&mutex_);
  283. FinishErase(table_.Remove(key, hash));
  284. }
  285. void LRUCache::Prune() {
  286. MutexLock l(&mutex_);
  287. while (lru_.next != &lru_) {
  288. LRUHandle* e = lru_.next;
  289. assert(e->refs == 1);
  290. bool erased = FinishErase(table_.Remove(e->key(), e->hash));
  291. if (!erased) { // to avoid unused variable when compiled NDEBUG
  292. assert(erased);
  293. }
  294. }
  295. }
  296. static const int kNumShardBits = 4;
  297. static const int kNumShards = 1 << kNumShardBits;
  298. class ShardedLRUCache : public Cache {
  299. private:
  300. LRUCache shard_[kNumShards];
  301. port::Mutex id_mutex_;
  302. uint64_t last_id_;
  303. static inline uint32_t HashSlice(const Slice& s) {
  304. return Hash(s.data(), s.size(), 0);
  305. }
  306. static uint32_t Shard(uint32_t hash) {
  307. return hash >> (32 - kNumShardBits);
  308. }
  309. public:
  310. explicit ShardedLRUCache(size_t capacity)
  311. : last_id_(0) {
  312. const size_t per_shard = (capacity + (kNumShards - 1)) / kNumShards;
  313. for (int s = 0; s < kNumShards; s++) {
  314. shard_[s].SetCapacity(per_shard);
  315. }
  316. }
  317. virtual ~ShardedLRUCache() { }
  318. virtual Handle* Insert(const Slice& key, void* value, size_t charge,
  319. void (*deleter)(const Slice& key, void* value)) {
  320. const uint32_t hash = HashSlice(key);
  321. return shard_[Shard(hash)].Insert(key, hash, value, charge, deleter);
  322. }
  323. virtual Handle* Lookup(const Slice& key) {
  324. const uint32_t hash = HashSlice(key);
  325. return shard_[Shard(hash)].Lookup(key, hash);
  326. }
  327. virtual void Release(Handle* handle) {
  328. LRUHandle* h = reinterpret_cast<LRUHandle*>(handle);
  329. shard_[Shard(h->hash)].Release(handle);
  330. }
  331. virtual void Erase(const Slice& key) {
  332. const uint32_t hash = HashSlice(key);
  333. shard_[Shard(hash)].Erase(key, hash);
  334. }
  335. virtual void* Value(Handle* handle) {
  336. return reinterpret_cast<LRUHandle*>(handle)->value;
  337. }
  338. virtual uint64_t NewId() {
  339. MutexLock l(&id_mutex_);
  340. return ++(last_id_);
  341. }
  342. virtual void Prune() {
  343. for (int s = 0; s < kNumShards; s++) {
  344. shard_[s].Prune();
  345. }
  346. }
  347. virtual size_t TotalCharge() const {
  348. size_t total = 0;
  349. for (int s = 0; s < kNumShards; s++) {
  350. total += shard_[s].TotalCharge();
  351. }
  352. return total;
  353. }
  354. };
  355. } // end anonymous namespace
  356. Cache* NewLRUCache(size_t capacity) {
  357. return new ShardedLRUCache(capacity);
  358. }
  359. } // namespace leveldb