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

Moving to NATS instead of dragonboat due to it's inefficiencies. #11

Merged
merged 9 commits into from
Sep 25, 2022

Conversation

maxpert
Copy link
Owner

@maxpert maxpert commented Sep 25, 2022

I absolutely want to document this because this is a pretty major change, and it's for good reason. I recently filed a bug on dragonboat implementation on how I was able to crash state machine due to bug in the code base while it claims itself to be PRODUCTION READY. I tried visiting their Gitter, looked at the code from various aspects:

  • One thing that stood out very obvious to me was the fact that the code is actually not really clean, and it was forcing me to not focus my logic on core, and write tests on my own code because of all the nuances and fights I had to put up with specifics. I guess it's the abstraction that bothered me.
  • It was using PebbleDB for logs (when it could have gone for lighter alternatives) and the whole internal KV store for storing state-machine logs was internal in v3.x. v4.x claims to be beta, and given how it was absolutely killing me I am not willing to even go close to it.
  • No IPv6 support, I opened up ticket but seems like author is busy with other stuff, and this project is rather not being used in something serious to begin with.
  • SQLiteLogDB implementation was absolute nightmare to implement (In an effort to remove dependency on PebbleDB). I had to read the specifics of each function and implement the same KV based function on to SQLite based function. Now some of you might blame me for not sticking to PebbleDB, but I will lay it out right now I would rather have one storage engine than having multiple engines to deal with.

Given that and the issue here which led to look at NATS to begin with, it was very obvious to me that for streaming, NATS was doing everything I needed to reliably replicate change logs, it was matter of just kicking off the server, and then talking to it. Given that there was already interest on numerous forums that people wanted to get CDC as part of bigger system, I had to use NATS client anyway, and embedding NATS server is no big deal as well.

I benchmarked with NATS for couple of hours, and the maturing of project just spoke to me. This PR takes the ugly step of adding dependency on NATS which I from my heart believe will be good future of Marmot. I know people might hate me for adding one more moving piece, but that's what UNIX philosophy is; and I am pretty sure just like me getting impressed by elaborate tooling, you will be impressed by how easy it will make your life as well.

So here is to a bright stable Marmot 🚀

@maxpert
Copy link
Owner Author

maxpert commented Sep 25, 2022

It's always good when you see so many lines deleted from code
image

@maxpert maxpert merged commit 851d6a1 into master Sep 25, 2022
@maxpert maxpert deleted the nats branch September 25, 2022 07:49
@lni
Copy link

lni commented Nov 29, 2022

Hi, dragonboat author here. I updated that issue with details. Link here (lni/dragonboat#256)

Since you mentioned some stuff about dragonboat, I'd like to offer my responses -

  • the problem you experienced was caused by your incorrect usage, details were given in the link above. Basically you injected byzantine fault into a typical non-byzantine system. The exact issue was actually explained in the document which you didn't read. You may want to review the literature to get yourself refreshed in that aspect.
  • rather than labelling the code as "not clean" without providing any specifics, I'd love to hear details, after all, it is an open source project looking for contributions, not finger pointings. thanks.
  • log storage will always be a part of the internal package, this is to stop users importing any details they are not suppose to access.
  • As immediately answered in your IPv6 support issue (IPv6 support lni/dragonboat#255), it will be nice to have that support. Given there is no IPv6 requirement from my side, I am looking for contributors like you to contribute PRs for such new features. Please note that it is an open source project, if you don't like certain things, fill a PR to get it changed or fixed, no response from the people maintaining it? fork it and do it.
  • Having SQLite to store the log is not a good idea, the performance is going to horrible. I know that as I have toy implementation using SQLite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants