Yingyi Bu created ASTERIXDB-1947: ------------------------------------ Summary: Revisit thread safety in FileMapManager Key: ASTERIXDB-1947 URL: https://issues.apache.org/jira/browse/ASTERIXDB-1947 Project: Apache AsterixDB Issue Type: Improvement Components: STO - Storage Reporter: Yingyi Bu Assignee: Abdullah Alamoudi
Synchronizations on FileMapManager (e.g., synchronized register/unregister, and ConcurrentHashMap for id2namemap and name2idmap) is added in change https://asterix-gerrit.ics.uci.edu/#/c/1840/. However, this class seems 50%-baked -- there are some synchronizations but there could be inconsistency between id2nameMap and name2IdMap. For example, register/unregister are not atomic -- a caller can lookupFileId(...) while the file hasn't fully registered or unregistered, and possibly open an non-existent file? I think we probably don't want thread-safety for FileMapManager at all, since the call sites anyway has to orchestrate multiple steps and make sure the bigger block is synchronized: E.g. BufferCache.openFile(...): {noformat} synchronized (fileInfoMap) { if (fileMapManager.isMapped(fileRef)) { fileId = fileMapManager.lookupFileId(fileRef); } else { fileId = fileMapManager.registerFile(fileRef); } openFile(fileId); } return fileId; {noformat} Even if FileMapManager is thread-safe, it doesn't help and you still have to synchronized on the big block to make sure atomicity of the sequence of isMapped/lookup/register. Most call sites have a similar pattern. Thoughts? -- This message was sent by Atlassian JIRA (v6.4.14#64029)