Plug hole that allowed concurrent replication streams

uber-archive / cherami-server

Distributed, scalable, durable, and highly available message queue system. This project is deprecated and not maintained.

https://eng.uber.com/cherami/

MIT License

1.42k stars 102 forks source link

Plug hole that allowed concurrent replication streams #323

Closed kirg closed 6 years ago

kirg commented 6 years ago

fix for concurrent replication streams
update the 'first seqnum' in replication code-path (this was missing)
added more logs around open/close extent

coveralls commented 6 years ago

Coverage increased (+0.3%) to 66.936% when pulling f7189ba803112598f94b19b90f9464d77f6b6bba on storehost-replication into 65a1bb1404b63ce07025a2a3f6d34187fddce030 on master.

thuningxu commented 6 years ago

Can you add unit test?

kirg commented 6 years ago

There is a unit-test that checks to see that when two OpenReplicateStream requests come in, only one will succeed and the other will fail. Unfortunately, this bug shows up when a third OpenReplicateStream request comes in!

coveralls commented 6 years ago

Coverage increased (+0.3%) to 66.895% when pulling d1b5c4df9a1dc9dd53b78d598e061fdff37f03fe on storehost-replication into 65a1bb1404b63ce07025a2a3f6d34187fddce030 on master.

coveralls commented 6 years ago

Coverage increased (+0.1%) to 66.763% when pulling 2ef7258288103c4b5040d1433fe29f9b099f5768 on storehost-replication into 65a1bb1404b63ce07025a2a3f6d34187fddce030 on master.

kirg commented 6 years ago

@thuningxu I have updated the existing test to try another (third) replicate-extent request, which should fail; verified that it fails without fix and passes with.