davisp commented on a change in pull request #610: Optimize ddoc cache URL: https://github.com/apache/couchdb/pull/610#discussion_r125698313
########## File path: src/ddoc_cache/src/ddoc_cache_lru.erl ########## @@ -0,0 +1,293 @@ +% Licensed under the Apache License, Version 2.0 (the "License"); you may not +% use this file except in compliance with the License. You may obtain a copy of +% the License at +% +% http://www.apache.org/licenses/LICENSE-2.0 +% +% Unless required by applicable law or agreed to in writing, software +% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +% License for the specific language governing permissions and limitations under +% the License. + +-module(ddoc_cache_lru). +-behaviour(gen_server). +-vsn(1). + + +-export([ + start_link/0, + open/1, + refresh/2 +]). + +-export([ + init/1, + terminate/2, + handle_call/3, + handle_cast/2, + handle_info/2, + code_change/3 +]). + +-export([ + handle_db_event/3 +]). + + +-include("ddoc_cache.hrl"). + + +-record(st, { + pids, % pid -> key + dbs, % dbname -> docid -> key -> pid + size, + evictor +}). + + +start_link() -> + gen_server:start_link({local, ?MODULE}, ?MODULE, [], []). + + +open(Key) -> + try ets:lookup(?CACHE, Key) of Review comment: That's all mostly right. The motivation for this was that it would allow the individual entries to be direct lookups rather than table scans and also allows for the non-rev-specific entry to automatically update its value when a design document is updated. As to the extra non-cache lookups the way I reasoned about it was that its only a constant number in the frequently asked case so once they're in cache its not an issue. And on the flip side if we're iterating over things in such a way that if we're evicting entries and then have a lot of traffic to un-cached entries it doesn't matter either as we'd have to increase the cache size regardless. I'll take a ponder on how to insert things into the cache from non-client requested actions but it seems to me like it'd open up a lot of race conditions in the already fairly complex logic around managing the cache entries. Also for the rev-specific entries that's true for the most part. They'll stick around until evicted or a quorum of their database shards are compacted. I'll contemplate how easily it'd be to add some sort of check there so that they'll close if they refresh and are not the most recent revision. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
