ssbc / ssb-db2

A new database for secure-scuttlebutt
47 stars 8 forks source link

use monotonic-timestamp in create() #376

Closed staltz closed 1 year ago

staltz commented 2 years ago

ssb-db and ssb-validate both use monotonic-timestamp to avoid the problem where two consecutive calls of Date.now() would give the same number. In our case this would help so we can avoid the Date.now() + 1, Date.now() + 2 hacks. I do think that the async-await we use in create() already guarantees that Date.now() will be different (I was unable to create a 1st :x: 2nd :heavy_check_mark: test), but it makes sense to anyway use monotonic-timestamp to be sure we never bump into this problem.

staltz commented 2 years ago

I'm not sure what's going on with the tests, they pass locally, but we definitely need to fix them in CI before merging.

staltz commented 2 years ago

The reason why tests were broken was that monotonic-timestamp was being replaced by mock-monotonic-timestamp (because the way we use ssb-fixtures ends up "leaking" this hack globally) so that there was a test that checked that the msg.timestamp should be greater than some Date.now(), but the msg.timestamp was in the year 2015, because of mock-monotonic-timestamp.

I used pnpm in CI, which fixed that error, but then other problems came up: some timestamps are floats, like 1659105426666.001 which doesn't work in bendybutt because ssb-bfe only bypasses integers, but also it turns out that bencode only supports integers, not floats. The JS library bencode will convert the float to an integer.

@arj03 What do you think, should we quit this PR?

UPDATE: I guess the tests passed this time randomly, but we're sure that sometimes we'll get a float timestamp and bendybutt will break.

github-actions[bot] commented 2 years ago

Benchmark results

Part Duration
Create 5000 new messages 560.51ms
Validate 5000 messages 639.76ms
Native to db format 5000 messages 153.66ms
Db to native format 5000 messages 118.68ms
Add 1000 elements 542.95ms
Add 1000 box1 msgs 1326.61ms
Unbox 1000 box1 msgs first run 246.76ms
Unbox 1000 box1 msgs second run 143.90ms
Add 1000 box1 msgs 1274.90ms
Query 1000 msgs first run 47.61ms
Query 1000 msgs second run 29.09ms
Add 1000 box2 msgs 1904.28ms
Unbox 1000 box2 msgs first run 326.09ms
Unbox 1000 box2 msgs second run 247.98ms
Migrate (+db1) 16266.35ms
Migrate (alone) 6052.15ms
Migrate (+db1 +db2) 12551.21ms
Migrate (+db2) 8632.67ms
Migrate continuation (+db2) 1353.85ms
Memory usage without indexes 754.91 MB = 39.51 MB + etc
Initial indexing 885.96ms
Initial indexing maxcpu=86 4893.51ms
Initial indexing compat 1120.10ms
Two indexes updating concurrently 1351.57ms
Key one initial 52.87ms
Key two 1.52ms
Key one again 1.38ms
Reboot and key one again 66.90ms
Latest root posts 1086.30ms
Latest posts 15.28ms
Votes one initial 725.03ms
Votes again 1.15ms
HasRoot 494.84ms
HasRoot again 0.37ms
Author one posts 644.45ms
Author two posts 21.04ms
Dedicated author one posts 641.23ms
Dedicated author one posts again 0.70ms
DeleteFeed 3957.05ms
Maximum memory usage 1029.91 MB = 65.60 MB + etc
Indexes folder size 10.01mb
arj03 commented 2 years ago

Regarding bendy butt I guess it's a matter of interpretation. I really dislike that we allowed floating timestamps in classic and think it would be better overall to convert all of them to int.

staltz commented 2 years ago

So does that mean we should make a new (version or fork) monotonic-timestamp that only gives integers?

arj03 commented 2 years ago

That would make this PR a lot more tasty :)

staltz commented 1 year ago

Hmm, pretty sure we could rewrite a new PR instead of working on this old one. I want to kill some old PRs.