Blin 2020.08, round 1 #3850
Comments
Altai-man
added
regression
|
I can't replicate the BSON failure. I run the test in a loop and see: but with different values for The failure doesn't really make sense at all. If I understand the BSON code and tests correctly. The test is this: assigned earlier here constructed like this which I think means that it hit this code: so the test failing seems to mean that Which makes no sense to me. |
|
I can replicate it reliably: It seems that it creates an ObjectId without arguments, which populates |
|
I can't at all, which makes it rather hard for me to figure out what to do next. Which OS and which compiler were you testing with? The only hint as to what the cause might be is that for the results you paste,
|
Blin uses debian image, I think, and could detect the same issue. |
|
Not a regression. (still a bug). I can replicate it on 2020.07: but I have to hack MoarVM to fake PIDs that are >65536: I will try to figure out the case of the bug, but I don't think that it is a blocker. |
Cannot reproduce it. It starts to become weird. Am I and blin the only beings who see this? Are there some conditions necessary for it to work, why I don't get them while trying out many times? |
|
It is weird, but it will fail if the system it is running on happens to produce process IDs > 65535, and I don't know what OSes do that, and whether (for them) process IDs are sequential (so that long running machines end up getting into this state) or randomised (so that certain OSes immediately (mostly) start generating large process IDs. But for the systems I 've tried, process IDs are consistently <= 65535, but I can replicate it if I patch more (as shown above) to make the return values from So, I then patched the test: and I now see this: line 258 is that last Chasing through the code
and that is truncating process IDs to two bytes, even when they are larger than two bytes. The code (or the regression tests) wrongly assume that process IDs are always <= 65535. I don't know why, and I don't think that it's essential for "BSON" that it stores the process ID. The spec that that test links to -- https://docs.mongodb.com/manual/reference/method/ObjectId/ -- says
so what the spec has as "5-byte random value" the BSON code is implementing as "3 byte machine ID" followed by "2 byte process ID". Which is fine (as long as it's random enough) but clearly doesn't "quite" pass its own tests when process IDs are larger than 2 bytes. Ooops. But it's only the test that are failing - truncating the PID to 2 bytes is still conformat with that spec. (What I do not know is whether that spec has changed, and it used to say "machine ID" and "PID", and then 10gen got a rude awakening when someone reported that PIDs can be large) |
|
Marking Data::Dump as done, as there is a PR available and the overall changes seem sensible. PID above 65535 is easily possible in Linux, see https://stackoverflow.com/a/6294196/6700717
This seems like the module's fault. I wonder how was it not exposed earlier, this sounds just unbelievable. And the second time this module got bisected to a completely different commit... Thanks for your time taken to do the investigation, @nwc10 ! Closing. |
Did an issue get opened on the module linking to this information? |
|
@jnthn MARTIMM/BSON#32 <- sure, I wouldn't close it otherwise. |
See MARTIMM#32 and rakudo/rakudo#3850 (comment) for more details, but in essence: This library has split the ObjectID 5-byte "random value" field as described in https://docs.mongodb.com/manual/reference/method/ObjectId/ into two subfields: a 3-byte machine name hash, and a 2-byte process ID. Unfortunately since most modern Linux/*nix systems use larger process IDs, this simply takes the first two bytes of the actual PID. That would not in itself be a problem, except that the tests check that the actual PID can be extracted back out of the ObjectID; these tests fail if the real PID happens to be larger than 65535. Since the spec doesn't actually *require* any structure to the "random value" field, just stop testing for this round tripping.
Blin results between 2020.07 (d0233dd) and HEAD (9d6d8dd):
Data::Dump – Fail, Bisected: c11f4b1
Old Output
New Output
BSON – Fail, Bisected: 9d6d8dd
Old Output
New Output
This run started on 2020-08-16T17:27:03Z and finished in ≈8 hours.
The text was updated successfully, but these errors were encountered: