_addOperation calls are serialized

[tabcat]

orbitdb/orbitdb#737

adds a queue to _addOperation()
close() awaits for the queue to empty before resolving

[tabcat]

pretty sure i made a serious mistake in the original commit that would have made the queue per all store instances instead of queue per store instance.

[vaultec81]

Can you add tests?
I think it should be good to go aside from additional review.

[tabcat]

i wasn't sure of a good way to test this so i never added one.

[vaultec81]

I suggest making a simple test that runs the code as if it would be a end user, or checking to see if the results were as intended this._addOperations calls are processed sequentially.

[haadcode]

The queue will be on critical code path. Before adding it, merging the PR, we should understand how this effects performance.

Can you run the benchmarks and provide the results for comparison?

[tabcat]

no queue:

60s benchmark-add:

--> Average of 520.9 q/s in the last 10 seconds
511 queries per second, 31466 queries in 60 seconds (Oplog: 31466)

benchmark-load:

Starting IPFS...
DB entries: 1000
Writing DB...
writing took 2715 ms
Loading DB...
load took 1369 ms

with queue:

60s benchmark-add:

--> Average of 551.8 q/s in the last 10 seconds
554 queries per second, 32507 queries in 60 seconds (Oplog: 32507)

benchmark-load:

Starting IPFS...
DB entries: 1000
Writing DB...
writing took 2462 ms
Loading DB...
load took 1260 ms

they were pretty back and forth

[tabcat]

without queue:

benchmark-add ran 60s 5 times:

1: 33193 queries in 60 seconds
2: 32637 queries in 60 seconds
3: 32104 queries in 60 seconds
4: 33240 queries in 60 seconds
5: 32460 queries in 60 seconds
avg: 32726.8

with queue:

benchmark-add ran 60s 5 times:

1: 30718 queries in 60 seconds
2: 32113 queries in 60 seconds
3: 32138 queries in 60 seconds
4: 32232 queries in 60 seconds
5: 32136 queries in 60 seconds
avg: 31867.4

avg performance difference with queue: -2.7%

[haadcode]

Thanks @tabcat, great to see the numbers! I'd be ok with that performance overhead.

[tabcat]

@haadcode do you think something like this best implemented here or left for user space?
the third option would be making it optional which i like less than both former options but im also not opposed.

[aphelionz]

@tabcat sorry, maybe I'm being slow... What are the three options?

[tabcat]

sorry

1. don't implement queue
2. implement queue
3. make queue optional per store

[aphelionz]

I think @haadcode's comment was an approval, though not explicit.

I'm ok with merging this in as well

[aphelionz]

@tabcat Merged, congrats. Can you run the tests on orbit-db proper to make sure everything works?

[tabcat]

yes they are all passing, i wish i would have thought to run them before now, my mistake. i only ran the orbit-db-store tests prior to this.
im sure you ran them both as well before merging, unfortunately i was afk at the time.

[aphelionz]

Great!