Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Master Task] Reduce locking on read path #139

Open
yiwu-arbug opened this issue Jan 31, 2020 · 0 comments
Open

[Master Task] Reduce locking on read path #139

yiwu-arbug opened this issue Jan 31, 2020 · 0 comments
Assignees
Labels
component/titan Component: Titan engine sig/engine Component: Engine (SIG)

Comments

@yiwu-arbug
Copy link
Collaborator

yiwu-arbug commented Jan 31, 2020

Currently for a point-lookup, Titan acquire 7 locks:

  • rocksdb db_mutex: to get snapshot
  • table cache mutex: to get block-based table reader
  • block cache mutex: to read blob index
  • titan db mutex: to get pointer to BlobStorage
  • BlobStorage mutex: to get blob file metadata
  • BlobFileCache mutex: to get file reader
  • blob cache mutex: to read cached blob

Compare with vanilla rocksdb, only 2 locks are acquired:

  • table cache mutex: to get block-based table reader
  • block cache mutex: to read blob index

It gives opportunities to optimize for CPU bounded scenarios. For example, by just reordering blob cache access to before getting blob file metadata, we get 6% throughput improvement in titandb_bench of in-memory workload. Here we list some of the ideas to reduce locking overhead:

Move blob cache access to BlobStorage level

#140

Currently we check blob cache in BlobFileReader. If we move the check to BlobStorage level, before BlobStorage::FindFile call, we can avoid BlobStorage mutex lock and BlobFileCache mutex lock for cache hit case.

The change is safe because, when we read a blob index from rocksdb, we assume the blob file it points to always exists (otherwise we should return Status::Corruption). So if we have a blob cache hit, we don't need to check blob file existence.

Move BlobFileCache check to before BlobStorage::FindFile call

Similarly, we can query BlobFileCache to get blob file reader, and if it's a hit, skip BlobStorage::FindFile call. That way for a cache hit (which is common) we saves a BlobStorage mutex lock.

Store BlobStorage pointer in ColumnFamilyHandle

rocksdb embed cfd pointer in ColumnFamilyHandleImpl. One of the benefit is, with the handle, a read doesn't need to acquire db_mutex to obtain cfd pointer. Similarly, we can embed BlobStorage pointer in the handle to save mutex lock to obtain BlobStorage pointer. We can define TitanColumnFamilyHandle as following and return it to caller. The struct is safe to be used as rocksdb::ColumnFamilyHandle when calling rocksdb methods.

struct TitanColumnFamilyHandle : public rocksdb::ColumnFamilyHandleImpl {
    std::shared_ptr<BlobStorage> blob_storage;
};

Avoid use weak_ptr

In the code we use the pattern a lot:

std::weak_ptr<Something> FindSomthing() {
    ...
    return ptr; // ptr is a std::shared_ptr
}

std::shared_ptr<Something> something = FindSomething().lock();

This is not necessary. We can have FindSomething return shared_ptr. Using weak_ptr have no benefit, and incur one more atomic ref-count operation.

Making rocksdb::DBImpl::GetSnapshot lock-free

It is also possible to make rocksdb GetSnapshot lock-free, though that's very involving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/titan Component: Titan engine sig/engine Component: Engine (SIG)
Projects
None yet
Development

No branches or pull requests

2 participants