davisp commented on a change in pull request #610: Optimize ddoc cache URL: https://github.com/apache/couchdb/pull/610#discussion_r129069864
########## File path: src/ddoc_cache/src/ddoc_cache_tables.erl ########## @@ -0,0 +1,68 @@ +% 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_tables). +-behaviour(gen_server). +-vsn(1). + + +-export([ + start_link/0 +]). + +-export([ + init/1, + terminate/2, + handle_call/3, + handle_cast/2, + handle_info/2, + code_change/3 +]). + + +-include("ddoc_cache.hrl"). + + +start_link() -> + gen_server:start_link({local, ?MODULE}, ?MODULE, [], []). + + +init(_) -> + BaseOpts = [public, named_table], + CacheOpts = [ + set, + {read_concurrency, true}, + {keypos, #entry.key} + ] ++ BaseOpts, + ets:new(?CACHE, CacheOpts), + ets:new(?LRU, [ordered_set, {write_concurrency, true}] ++ BaseOpts), + {ok, nil}. + + +terminate(_Reason, _St) -> + ok. + + +handle_call(Msg, _From, St) -> + {stop, {invalid_call, Msg}, {invalid_call, Msg}, St}. Review comment: You're quite right that it should be the client that terminates. I've always wanted to have some sort of VM feature that would allow us to define a protocol of message patterns a process was willing to receive. However I haven't got a clue how that'd work when it comes to distributed Erlang. However, given that something like that isn't available the next best possibility at enforcing a protocol is to bail if we get something unexpected. In some cases it can be appropriate to ignore messages, however given that this is a concurrent cache of design documents in a database it would be a bad idea. If we're getting messages that are unexpected then it means that something we've written is not behaving as intended. And given that this cache handles things like validate doc update functions it could be a sever issue if we were to have something working incorrectly. The one_for_all is legacy from a previous iteration of this work that split the LRU work between two gen_servers. In that case if either of the servers died for any reason we would have wanted the other to also be restarted so that state is not inconsistent. Now that all the eviction and opening logic is in a single gen_server I'll remove the tables process and revert to a standard one_for_one supervisor strategy. ---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services
