-
Notifications
You must be signed in to change notification settings - Fork 24
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
Discuss: BEP-10 #38
Comments
I have certain concerns about the motivation of the BEP.
We might want to investigate how to replace Tendermint's storage backend. It feels big and probably deserves a separate BEP. It might involve:
Approaching A is likely worse performance-wise compared to the tcp connections created in advance for ABCI. Do you consider Approach B to be more complicated even if we rewrite ABCI method by ABCI method keeping the not yet migrated ABCI calls go through py-abci?
What prevents us from consolidating this logic independently from this BEP?
What are the exact issues we face here? If we have only 2 processes (
Do we have some rough ideas about how to unify the configuration? Should it be a different PEP? In general, getting rid of py-abci comes at a cost of maintaining our own integration with Tendermint. The integration itself should be faster and easier to maintain. The reimplemented transaction validation logic which is tightly coupled to the integration itself will be also faster but subjectively harder to maintain since Go is new to the team and offers fewer of the high-level programming language features. |
This BEP starts out by listing several issues with the current situation, but it doesn't try (or claim to try) to solve them all in one BEP. Replacing LevelDB with MongoDB would definitely be the topic of a future and separate BEP, if we ever do it at all. The process of writing the BEP would help us weigh the pros and cons. Right now, it's not obvious to me that using MongoDB-only would be better. The refactoring of the transaction validation code will also be handled by other BEPs, starting with BEP-9. For now, we'll just add a new HTTP API endpoint that the Go code can call to ask BigchainDB to check the validity of the passed transaction. That is, we won't convert any transaction validation code to Go as part of this BEP. The Tendermint team already maintains a framework/starting point framework for writing an "ABCI application" (like BigchainDB) in Go, so we could use that code as a starting point, i.e. we can just import it. |
Basically, I am trying to assemble the particular issues this BEP will try to solve. So far I can clearly see two of them: the performance and the ease of maintenance. As for the performance, do we have a particular bottleneck? Or do we want to sqeeze the maximum out of a particular metric (like a transaction rate) so that we try to measure it while implementing the BEP? Another problem is, the approach A (the new HTTP API endpoint), while being a reasonable temporary solution, is likely to compromise the performance a little and thus spoil our possible measurements. Also, I have doubts about the maintenance effort. It seems straightforward to do the initial integration to talk to the new HTTP endpoint, indeed. But the further development might come at a bigger cost compared to the Python path, even considering what will come after we rewrite what we have. |
In my opinion, this BEP is mostly about replacing We don't want to spend time maintaining or developing code that's not central to what BigchainDB is. In this case, it's the ABCI interface code, which is a Tendermint-specific thing. Let Tendermint take care of that. |
Can you post a link to what you are referring to as the Go equivalent? I've found some simple examples (including an example of a Python ABCI server by the way) there. We can use one of them but we still need to implement a custom startup script and the HTTP communication - an HTTP server instead of an ABCI server is not something they seem to offer. I understand the maintenance benefit of turning the ABCI server into the HTTP server. It is straightforward to implement, only requires a little bit of Go, and hopefully (!) does not hurt the performance too much. If this is what we want to do, then it's a simple and independent task, not the strangler approach to the application rewrite. |
The Tendermint Ecosystem page in the Tendermint docs lists several ABCI Server implementations, including the Go one under the tendermint/abci repo. That one seems a bit old. I think the reason is that they've been focused on a Go-based Tendermint application development toolkit called "Cosmos-SDK" which includes the ABCI Server plus a whole pile of other useful stuff. We don't have to use the stake stuff; I gather that that's an optional plugin. |
@ldmberman Following are some responses to your concerns
As you mentioned its a huge task and deserves its own BEP. Our initial investigation revealed that the Tendermint's backend logic is consolidated and there is a golang MongoDB module which could be used for reference.
Couldn't understand the concern clearly but neither of those approaches (A and B) made it to the final design. A hybrid approach which takes some parts of A and B has been adopted.
The concern of this BEP is not re-factoring but rather moving to an officially supported ABCI library.
The major pain point is the interation between ABCI proxy app and Tendermint. Some design decisions by Tendermint team have made this communication channel less fault tolerant. It would always be better option for us to choose an implemtation that can avoid using this communication channel.
I don't think there is a easy and clean of merging BigchainDB and Tendermint's configuration. In my humble opinion it will lead to more mess. |
The idea of merging the BigchainDB and Tendermint configuration settings (or not) can also be deferred to another BEP. One of the reasons for BEPs is to force us to think clearly and completely about the options, and the pros and cons of each. (They have other purposes too, such as being open and transparent, giving coders something to implement, giving testers something to test, and giving documentation-writers a good starting point.) |
There does not seem to be such a thing as officially supported ABCI library. Does not prevent us from getting rid of the ABCI socket connections though. So far I take it as the goal of the BEP is to get rid of the ABCI socket connections. I think, we should rename the BEP to reflect it and clarify its motivation a little. I can come up with a PR. |
For Go, I think the officially supported library/SDK is Cosmos-SDK. For JavaScript, I think it's LotionJS. |
I would consider the PS: IIRC |
I do not exactly see yet how Cosmos-SDK can help us achieve the goal. My point is, our problem is not the library, otherwise we would just use the Python example from the |
Correct me if I am wrong but there are no tests for the python example code which you are refering to. I also don't see clearly how production ready that python code is. To me it seems that we can't just use that Python example code and expect BigchainDB to be stable. We would have to write substantial tests to make it production ready, ensure that the Python code is kept up to date with latest Tendermint's wire format etc. So, IMHO the library is the problem to quite an extent. |
I agree, using that example would likely bring issues to the table - I mentioned it because it is a part of an "official library". To implement the BEP, we need to write some custom Go code based solely on the Tendermint itself as a library - a custom startup script and an |
@vrde @ttmc @ldmberman One performace issue I see with the current adopted approach is follows,
To me it seems that there will be a lot of repeated data flow because each block will have hundereds of transactions. |
To sum up the offline discussion we had with @kansi @vrde @ttmc @z-bowen @muawiakh @shahbazn @codegeschrei: One of the main goals of the BigchainDB platform is performance. We aim to get as big of the transaction rate as possible. We anticipate the rewrite of BigchainDB in Go to bring significant performance gains on various fronts. BEP-10 offers a reasonable first step towards the goal but since there is a huge maintenance tradeoff in it we want to properly benchmark BigchainDB to gain enough confidence in making this tradeoff. Additionally, we want to isolate certain pieces of the platform, prototype alternative implementations of those pieces in Go, and compare them. Specifically, we want to try the following (note that some of the bullet points are independent):
|
No description provided.
The text was updated successfully, but these errors were encountered: