Open LMDB files with MDB_NOLOCK if no write access

[lukeyeager]

Similar to [the first half of] #2384. The difference is that MDB_NOLOCK is only used if you were unable to acquire the lock, and a WARNING will appear in the logs.

/cc @cypof @hyc

[longjon]

I think if we're going to allow this kind of behavior, it should be explicitly specified by the user, no? (This is still subject to @hyc's concerns, right?)

Allowing the user to demand something dangerous is okay; falling back to dangerous behavior isn't, IMO, log messages notwithstanding.

[longjon]

In fact I've been running into this myself lately, so I think I'm favor of having this functionality, but as an explicit (build?) option.

[lukeyeager]

I think I'm favor of having this functionality, but as an explicit (build?) option.

Ok, build option added. Does ALLOW_LMDB_READONLY sound like the right name?

I also rearranged a few lines in the build files from #2523 and #3081 to standardize on the order USE_OPENCV, USE_LEVELDB, USE_LMDB and ALLOW_LMDB_READONLY. I did that mostly so that I could put the new option right below USE_LMDB.

[longjon]

"Allow readonly" doesn't feel right to me; you're always allowed to read a read-only LMDB. The issue is whether you're allowed to open an LMDB without locking (and indeed, the flag in question is MDB_NOLOCK). ALLOW_LMDB_NOLOCK seems more appropriate to me. The Makefile.config comment also needs to explain the issue (i.e., you should not set this flag if you will be reading LMDBs with any possibility of simultaneous read and write).

Rearranging lines is fine, but let's have a separate commit for that.

[lukeyeager]

ALLOW_LMDB_NOLOCK seems more appropriate to me

Haha ok that's what I started with initially but I changed my mind. Changed it back.

The Makefile.config comment also needs to explain the issue

Done.

Rearranging lines is fine, but let's have a separate commit for that.

Done.

[longjon]

Looks good except:

• The default seems to be different between CMake and Makefile; should it be on or off? (I guess I'm in favor of off as the more conservative option.)
• Is it necessary to call mdb_env_close and recall mdb_env_create if mdb_env_open fails? That seems odd to me but I'm not that familiar with the LMDB API.

[lukeyeager]

The default seems to be different between CMake and Makefile; should it be on or off? (I guess I'm in favor of off as the more conservative option.)

They both default to OFF/0 already. I didn't add the ALLOW_LMDB_NOLOCK ?= 1 line.

Is it necessary to call mdb_env_close and recall mdb_env_create if mdb_env_open fails? That seems odd to me but I'm not that familiar with the LMDB API.

Seemed strange to me too. I think it gave me an error. Either way, it's what the docs say to do:

If this function fails, mdb_env_close() must be called to discard the MDB_env handle.
http://symas.com/mdb/doc/group__mdb.html#ga32a193c6bf4d7d5c5d579e71f22e9340

[longjon]

They both default to OFF/0 already. I didn't add the ALLOW_LMDB_NOLOCK ?= 1 line.

Oops, I misread it.

This seems pretty ready to me, I'll leave it momentarily to see if there is further discussion, but then I think we can merge.

[lukeyeager]

This seems pretty ready to me, I'll leave it momentarily to see if there is further discussion, but then I think we can merge.

Ping.

[longjon]

Sorry for the wait! This looks good to me, merging.

Thanks @lukeyeager, this provides a nice workaround for a common annoyance.