Bug 1644359: Support error reporting in Python bindings #1039
Conversation
mdboom
force-pushed
the
python-upload-error-reporting
branch
from
a68436f to
54727fd
Compare
| data_dir: FfiStr, | ||
| language_binding_name: FfiStr, | ||
| ) -> u8 { | ||
| pub unsafe extern "C" fn glean_initialize_for_subprocess(cfg: *const FfiConfiguration) -> u8 { |
There was a problem hiding this comment.
What happens if two processes write/open the database at the same time? Thin of the 'pingsender' case on Windows: FOG might be still running while the pingsender is being used.
There was a problem hiding this comment.
My understanding is that LMDB handles this for us. See the Threads and Processes section here.
What isn't supported is opening the DB multiple times from the same process.
There was a problem hiding this comment.
That's very interesting. Would you mind adding a small comment about this guarantee here or in some other place? It would also be great to have test coverage for this, but I guess it's fine to do this as a follow-up (or at least just file a bug)
There was a problem hiding this comment.
The new Python tests in this PR are already sort of testing this case. Even in test mode, it fires off a subprocess (the difference is that it "joins" and waits for it to finish). To be extra sure, we could write a test that repeatedly changes metrics while the ping uploader is running and reporting network errors... Should be easy enough, so I might as well do that now.
There was a problem hiding this comment.
I was wrong about testing mode still using real subprocesses. So good catch -- I updated the tests to actually do that so we are testing things running on separate processes, and also added one that sets metrics from both processes at the same time.
mdboom
force-pushed
the
python-upload-error-reporting
branch
4 times, most recently
from
1dc1f88 to
fa693d8
Compare
This works by initializing a full Glean in the subprocess that *doesn't* do any startup work such as setting metrics or submitting pings. This makes it safe to initialize Glean in the subprocess, and thus errors can be reported there.
mdboom
force-pushed
the
python-upload-error-reporting
branch
from
fa693d8 to
14d9928
Compare
This works by initializing a full Glean in the subprocess that
doesn't do any startup work such as setting metrics or submitting
pings. This makes it safe to initialize Glean in the subprocess,
and thus errors can be reported there.