[ 
https://issues.apache.org/jira/browse/HBASE-18214?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16061660#comment-16061660
 ] 

Devaraj Das edited comment on HBASE-18214 at 6/23/17 11:58 PM:
---------------------------------------------------------------

Looks fine but I think the lock inside {code}mapped_type& operator[](const 
key_type& key) {code} (in the class concurrent_map) should be the unique_lock 
since we are mutating the map when the key doesn't exist, no?


was (Author: devaraj):
Looks fine but I think the lock inside {code}mapped_type& operator[](const 
key_type& key) {code} should be the unique_lock since we are mutating the map 
when the key doesn't exist, no?

> Replace the folly::AtomicHashMap usage in the RPC layer
> -------------------------------------------------------
>
>                 Key: HBASE-18214
>                 URL: https://issues.apache.org/jira/browse/HBASE-18214
>             Project: HBase
>          Issue Type: Sub-task
>            Reporter: Devaraj Das
>            Assignee: Devaraj Das
>         Attachments: 18214-1-1.txt, 18214-1-2.txt, hbase-18214_v3.patch, 
> hbase-18214_v4.patch
>
>
> In my tests, I saw that folly::AtomicHashMap usage is not appropriate for 
> one, rather common use case. It'd become sort of unusable (inserts would 
> hang) after a bunch of inserts and erases. This hashmap is used to keep track 
> of call-Id after a connection is set up in the RPC layer (insert a 
> call-id/msg pair when an RPC is sent, and erase the pair when the 
> corresponding response is received). Here is a simple program that will 
> demonstrate the issue:
> {code}
> folly::AtomicHashMap<int, int> f(100);
> int i = 0;
> while (i < 10000) {
>     try {
>       f.insert(i,100);
>       LOG(INFO) << "Inserted " << i << "  " << f.size();
>       f.erase(i);
>       LOG(INFO) << "Deleted " << i << "  " << f.size();
>       i++;
>     } catch (const std::exception &e) {
>       LOG(INFO) << "Exception " << e.what();
>       break;
>     }
> }
> {code}
> After poking around a little bit, it is indeed called out as a limitation 
> here 
> https://github.com/facebook/folly/blob/master/folly/docs/AtomicHashMap.md 
> (grep for 'erase'). Proposal is to replace this with something that will fit 
> in in the above usecase (thinking of using std::unordered_map).



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to