Fixed test_mt and made TCPClient threadsafe #83
Conversation
|
@jcarreira I realized that the CacheManager is likely not thread safe at the moment due to the lack of locks on the cache itself. Assuming that having a thread safe CacheManager is a priority, would it be better to simply add a around the map used as the cache, or to convert the map into a cuckoo map or another thread safe map implementation? |
src/client/TCPClient.cpp
Outdated
| @@ -103,15 +104,18 @@ cirrus::Future TCPClient::write_async(ObjectID oid, const void* data, | |||
| auto msg_contents = message::TCPBladeMessage::CreateWrite(*builder, | |||
| oid, | |||
| data_fb_vector); | |||
| curr_txn_id_lock.wait(); | |||
There was a problem hiding this comment.
I don't think you need this. There are atomic types in C++
|
@jcarreira latest commit addresses your comment. |
| @@ -103,15 +105,16 @@ cirrus::Future TCPClient::write_async(ObjectID oid, const void* data, | |||
| auto msg_contents = message::TCPBladeMessage::CreateWrite(*builder, | |||
| oid, | |||
| data_fb_vector); | |||
| const int txn_id = curr_txn_id++; | |||
There was a problem hiding this comment.
As far as I understand, you use a map txn_id->txn_info to keep the state of a put/get operation. This is necessary because our network layer works as an event handler.
I think this needs to be briefly explained in the code. Probably in a comment to txn_map.
|
Latest commit adds an explanation, let me know what you think |
Fixes #45 . Changed the
test_mttest so that each thread works in its own range.Updated TCPClient so that it is thread safe.
The RDMA Client hangs in the new test, so I'm thinking there's some element that is not thread safe. I'll look into this further, but the changes elsewhere are still good to be looked over. I've opened an issue in #84