RobThree / IdGen

Twitter Snowflake-alike ID generator for .Net
MIT License
1.19k stars 148 forks source link

How to sync DefaultTimeSource's Elapsed #20

Closed Caskia closed 4 years ago

Caskia commented 4 years ago

I have two nodes writing records to database, using this component to generate the ids. But this ids' ordering is different from creationtime's, I think it's related to DefaultTimeSource. I can not ensure this nodes start at same time, so the Stopwatch's Elapsed is different. How can I solve this problem? Remove the stopwatch's Elapsing? I don't know what it does.

RobThree commented 4 years ago

Which version are you using? (See #18)

The nodes aren't (usually) supposed to know about eachother; however: to synchronise their clocks you could/should use NTP.

Caskia commented 4 years ago

I have read this post #18 before. version: 2.1.0

You can try this code:

var idGenerator1 = new IdGen.IdGenerator(10, new DateTime(2017, 12, 8, 0, 0, 0, DateTimeKind.Utc));

await Task.Delay(20 * 1000);
var idGenerator2 = new IdGen.IdGenerator(9, new DateTime(2017, 12, 8, 0, 0, 0, DateTimeKind.Utc));

var list = new List<(int, long)>();
list.Add((0, idGenerator1.CreateId()));
list.Add((1, idGenerator2.CreateId()));

await Task.Delay(1);

list.Add((2, idGenerator2.CreateId()));
await Task.Delay(5);
list.Add((3, idGenerator1.CreateId()));

the data in list index2 is always greater than index3

RobThree commented 4 years ago

Uh... you may have found a bug...

I'm currently looking into it. As a workaround, you may try this:

var ts = new DefaultTimeSource(new DateTime(2017, 12, 8, 0, 0, 0, DateTimeKind.Utc), TimeSpan.FromMilliseconds(1));

var idGenerator1 = new IdGen.IdGenerator(10, ts);
System.Threading.Thread.Sleep(20 * 1000);
var idGenerator2 = new IdGen.IdGenerator(9, ts);

This seems to ensure the same timesource is used.

RobThree commented 4 years ago

Hmm, ok, so here's a bit of undefined behaviour I hadn't thought about...

It took me a while to realize... Every IdGenerator shares the same, underlying, stopwatch which is used as a timesource. However, each IdGenerator gets it's own instance of a (Default)TimeSource. And those stored, internally, an offset (when it is created) which was specific to the instance but should be shared among instances too.

In some perspective it's 'correct' behavior but I don't think it's very intuitive (hence your issue). So I've fixed it in 3935623 and will push an update to NuGet shortly.

RobThree commented 4 years ago

I have pushed v2.2 to NuGet (may be a few minutes before it's available) should be available now. Please let me know if you still experience issues with this version.

Thanks for reporting this issue! 👍

Caskia commented 4 years ago

That's great, thank you for such a quick response, I will have a try.

Caskia commented 4 years ago

Why not use static constructor to start Stopwatch? Every DefaultTimeSource instances will start Stopwatch, although it will not have any impact.

RobThree commented 4 years ago

Because CA1810: Initialize reference type static fields inline 😉 Yes, the stopwatch is 'started' (actually 'resumed') every new instance but, as you said, it doesn't have any impact. If you have sound reasoning for moving to a static constructor I'm open for discussion 😉

Caskia commented 4 years ago

Thank you for giving me a lesson.

RobThree commented 4 years ago

You're welcome. But more importantly: Is your problem solved now? 😉

Caskia commented 4 years ago

it's solved 😄