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

Implement txid plugin to find transaction by hash #1904

Closed
wants to merge 2 commits into from

Conversation

abitmore
Copy link
Member

For client application developers who want to query for transaction with a hash directly from the node.

Added 2 API calls:

  • get_transaction_by_id ( txid, optional do_not_fetch_the_whole_tx )
  • get_transaction_count

Testing how much RAM it will use.

@abitmore abitmore added this to the 3.3.0 - Feature Release milestone Aug 12, 2019
@abitmore
Copy link
Member Author

Reached 12G RAM after

[by size: 20.26259% 14894044656 of 73505127655] [by num: 66.33646% 26550000 of 40023241]

All other plugins are disabled.

I don't think it's good to enable this plugin by default.

@bijianing97
Copy link

#1906

@abitmore abitmore marked this pull request as ready for review August 13, 2019 16:32
@pmconrad
Copy link
Contributor

Is there a specific reason for adding this? IMO keeping TX IDs in RAM is the wrong way. The plugin will be of little use for anyone.

IMO it would be better to add some kind of generic message streaming API that external applications can easily consume, as a common base for ES and other ways to store TX information in an external database.

@abitmore
Copy link
Member Author

There were quite some developers asked for an API to query for transaction info by txid. The same request pops up now and then. We didn't have one because we knew it would consume much RAM if implemented with boost multi-index container, as you said, it's not a good idea. This PR does prove it.

However, if someone needs the feature too badly and have lots of RAM to throw in, this plugin would be helpful. Actually I wrote this code because someone told me that they do need such a feature and "adding RAM is not an issue". I didn't know how much RAM we'll really need before get this done, as mentioned in OP. I don't know whether it's still "not an issue" for them after knowing the numbers.

At least, this PR defined the APIs. In the future if we decide to merge #1906 or implement something alike, we can update the APIs to read data accordingly (if user enabled this plugin, read from RAM; if enabled another plugin, read from elsewhere).

By the way, this is a really simple plugin, can be used by new developers to learn.

@abitmore abitmore force-pushed the pr-database-api-code-refactory branch 3 times, most recently from d51305c to 62fc622 Compare August 15, 2019 13:36
@abitmore abitmore changed the base branch from pr-database-api-code-refactory to develop August 15, 2019 22:04
@abitmore
Copy link
Member Author

abitmore commented Sep 4, 2019

#1957 will do the job. No one will use this. Closing.

@abitmore abitmore closed this Sep 4, 2019
@abitmore abitmore deleted the txid-plugin branch September 4, 2019 00:21
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.

4 participants