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
Read lock on LevelHandler while get #186
Comments
Yes, we currently perform the whole get process while holding read lock of
I think if we already get the table, it will not be dropped until nobody use it. |
It takes a little more time when do the real file drop and this will occur under write lock if nobody have the table ref. Don't you think doing drop under write lock might block the read process? |
I see, I misunderstood what you mean previously. Maybe we could just mark them as could be deleted and perform the real delete operation later. But I doubt if it is real expensive to delete a file... |
Well, it depends on CPU, OS, filesystem and disk performance. I just have a test on my cloud server. auto start = std::chrono::steady_clock::now();
remove("./64MiB");
auto end = std::chrono::steady_clock::now();
auto nano = std::chrono::duration_cast<std::chrono::nanoseconds>(end-start).count();
std::cout << nano << std::endl; |
When getting value from LSM, agatedb iters on levelhandlers and calls
LevelHandler::get()
under aRwLockReadGuard
which will cause the whole read I/O path on this level under a read lock. It is always not a good idea to do I/O under lock even it is a read lock.See:
agatedb/src/levels.rs
Line 931 in 475e17b
I think such read lock is only necessary for fetching associated Tables from
LevelHandler
and the exact read work can be done outside the lock.This logic is same as what it is in badger.
https://github.com/dgraph-io/badger/blob/7d159dd923ac9e470ffa76365f9b29ccbd038b0d/levels.go#L1602
The comment says it will call h.RLock() and h.RUnlock(), but exactly they are called in
getTableForKey()
which only locked when getting tables.https://github.com/dgraph-io/badger/blob/master/level_handler.go#L241-L243
BTW, as Table might be removed (I/O) when the struct drop and this might happened when modifying LevelHandler under write lock. So this might influence read? I' mot sure whether it's right?
The text was updated successfully, but these errors were encountered: