Couple of test failures due to mmap changes #212
Comments
|
Maybe it could fall back to file IO in this case? |
|
This seems like a problem specific to the AppVeyor instance, and may not happen for users trying out Badger on Windows. But I don’t have a Windows machine with me at the moment, to confirm. Moreover, falling back to File I/O would also mean maintaining two different code paths. Windows is not a primary target for us at the moment. We try to make sure that the code builds so that people can try out Badger on that platform. Maintaining a separate code path with File I/O fallback just for Windows doesn’t sound like a great option. Hopefully, we can figure out a workaround to get the build fixed. |
|
We could set the max value size to be the same as the size of log file (a parameter in options). Then we can set the write mmap to be twice the size of the log file. That way, we know that we won't exceed that limit. And it would also work on Appveyor. |
manishrjain
added
the
kind/maintenance
|
IPFS is failing because of this. Let's fix this asap. |
|
I don’t have a Windows machine, so I am using AppVeyor as a remote build bot to investigate. I created a small project. Here is the build history.
There is probably an issue in the way we mmap on Windows. This code was probably never actually exercised on Windows (in the I will investigate further. |
|
So after a lot of trial and error, I made some changes which work well in my test project replicating this problem. But the changes are still causing test failures (screenshot below):
Sigh… @manishrjain I hate to say it, but it seems like #235 might be the easiest way to get around this, after all.
|
|
Based on our last chat, a good solution here would be to not mmap until replay has been completed. Also, to preallocate the write file to 2xvalue log size, and then truncate it when done. |
mmap on Windows works differntly from mmap on Linux. * We cannot mmap to a size beyond the size of the file. So we need to truncate to that size and then map. * We cannot truncate a file while a section of it has been mapped. This has implications for how we manage value logs. This change fixes mmap-ing on Windows with the following changes: * When badger.NewKV() function is called, which in turn calls valueLog.Open() method, we don’t immediately mmap the writable log files (read-only log files are not a problem). We mmap the writable log after replay has happened. * Whenever the valueLog.iterate() method is called, and it determines that a file needs to be truncated - we need to either make sure that there are no open mmaps, or we need to munmap and then re-map the file after truncation. * Whenever a writable log file is mmap-ed, its size increases to ValueLogFileSize * 2 (because of truncation in y.Mmap() function). We need to ensure that once it has filled up, it is truncated back to the actual size before it is closed and opened as a read-only file. We also need to truncate if back if the value log is closed before it fills up. * Fix a few tests on AppVeyor that were running out of memory by reducing ValueLogFileSize option value. Fixes #212.
This seems to be happening because the AppVeyor instance does not have enough virtual address space to map the value log. I am guessing this should not prevent Badger from working on a Windows machine with enough RAM, but I don’t have one around to test.
The text was updated successfully, but these errors were encountered: