Skip to content
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

misc oids #431

Open
timotheecour opened this issue Dec 7, 2020 · 10 comments
Open

misc oids #431

timotheecour opened this issue Dec 7, 2020 · 10 comments
Assignees
Labels

Comments

@timotheecour
Copy link
Owner

timotheecour commented Dec 7, 2020

links

@timotheecour
Copy link
Owner Author

timotheecour commented Dec 7, 2020

var t = ...

proc genOid*(): Oid =
  ## Generates a new OID.
  t = getTime().toUnix.int32
  var i = int32(atomicInc(incr))
  ...

=>

let t = ...

proc genOid*(): Oid =
  ## Generates a new OID.
  var t = getTime().toUnix.int32
  var i = int32(atomicInc(incr))
  ...

@timotheecour
Copy link
Owner Author

timotheecour commented Dec 7, 2020

  • initialized on 1st use these:
var
  t = getTime().toUnix.int32
  seed = initRand(t)
  incr: int = seed.rand(0x7fff)

let fuzz = int32(seed.rand(high(int32)))

(in https://github.com/nim-lang/Nim/pull/16203/files)

@timotheecour
Copy link
Owner Author

instead maybe:

in var r = initRand(t)

@timotheecour
Copy link
Owner Author

  • add overload for genOid which accepts Rand as a parameter.

@timotheecour
Copy link
Owner Author

timotheecour commented Dec 7, 2020

EDIT: see https://github.com/mongodb/mongo/blob/master/src/mongo/bson/oid.cpp which uses:

SecureRandom entropy;
ounter = std::make_unique<AtomicWord<int64_t>>(entropy.nextInt64());
_instanceUnique = OID::InstanceUnique::generate(entropy);

=> SecureRandom should be ported and defined in std/random
see #429 (comment)

@timotheecour
Copy link
Owner Author

timotheecour commented Dec 7, 2020

  • var i = int32(atomicInc(incr)) will eventually overflow for 2 reasons: when atomicInc increases incr beyond int.high (can be 32 bits or 64 bits), and (on 64 bits) when incr goes beyond range of int32

instead, make sure there's no overflow by using uint throughout (and/or cast as needed)

note: can be seen with:

when defined case5:
  var incr: int = 0x7fff
  proc main =
    while true:
      var i = int32(atomicInc(incr))
  main()

which quickly overflows

note

mongodb oid makes no guarantee of increasing ids, so wrap-around behavior (instead of raising overflow error) is fine; see https://stackoverflow.com/questions/31057827/is-mongodb-id-objectid-generated-in-an-ascending-order

@timotheecour
Copy link
Owner Author

timotheecour commented Dec 7, 2020

  • IGNORE

controversial; EDIT: ignore for now since https://github.com/mongodb/mongo/blob/master/src/mongo/bson/oid.cpp also uses a random number not generated from processid/machine id

we should use what's recommended in https://www.mongodb.com/blog/post/generating-globally-unique-identifiers-for-use-with-mongodb
ObjectID is a 96-bit number which is composed as follows:

ObjectID is a 96-bit number which is composed as follows:

a 4-byte value representing the seconds since the Unix epoch (which will not run out of seconds until the year 2106)
a 3-byte machine identifier (usually derived from the MAC address),
a 2-byte process id, and
a 3-byte counter, starting with a random value.

in particular, this needs to be implemented:

a 3-byte machine identifier (usually derived from the MAC address),
a 2-byte process id, and

instead of:
let fuzz = int32(seed.rand(high(int32))) which is redundant with the last 3 byte counter

note:

another doc https://docs.mongodb.com/manual/reference/method/ObjectId/ just mentions a 5-byte random value but the 1st doc seems more useful

links

https://stackoverflow.com/questions/9924701/how-machine-id-host-name-is-mapped-demapped-to-3-byte-in-object-id

The ObjectID 3 byte machine field is the first three bytes of the (md5) hash of the machine host name, or of the mac/network address, or the virtual machine id. So, it can't be reversed back even if you wanted to.

EDIT:
see https://github.com/mongodb/mongo/blob/master/src/mongo/bson/oid.h#L70

> The process unique is an arbitrary sequence of 5 bytes. There are no endianness concerns
 * since it is never interpreted as a multi-byte value.

@timotheecour
Copy link
Owner Author

timotheecour commented Dec 7, 2020

  • parseOid needs tests (reverse of $)

@timotheecour
Copy link
Owner Author

timotheecour commented Dec 7, 2020

  • deprecate oidToString (unsafe!) and make sure $ doesn't use it directly (can be done without code duplication)

@timotheecour
Copy link
Owner Author

  • add justForked as in mongodb

https://github.com/mongodb/mongo/blob/02afb5b532bd893919b3cc2228fd62e958e91240/src/mongo/bson/oid.h#L70

 > * Warning: You MUST call OID::justForked() after a fork(). This ensures that each process will
 * generate unique OIDs.

@ringabout ringabout self-assigned this Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants