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.

Sign up for GitHub

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

Jump to bottom

add failed operation count to ledger resource #1385

Closed
tomerweller opened this issue Jun 11, 2019 · 7 comments · Fixed by #2690
Closed

add failed operation count to ledger resource #1385

tomerweller opened this issue Jun 11, 2019 · 7 comments · Fixed by #2690
Assignees
@abuiles
Labels
horizon horizon-api Issues or features related to the Horizon API
Milestone
Horizon 1.5.0

Comments

@tomerweller
Copy link
Contributor

The ledger resource currently reports operation_count which represents the number of successful applied in the ledger.

With protocol 11 changes it's difficult to asses the used capacity of a transaction set without a failed operation count.

I suggest deprecating operation_count and introducing successful_operation_count and failed_operation_count.

Note: failed operations might be semantically confusing. An operation result can be successful, but if it's within a failed transaction, the operation is not applied and therefore it has in fact failed. I don't think that's a sufficient reason not to introduce the failed operations metric, but it's important to document accordingly.
@bartekn bartekn mentioned this issue Jun 11, 2019
Merged
7 tasks
@bartekn
Copy link
Contributor

bartekn commented Jun 11, 2019

The name can be confusing, indeed. Maybe we should call it: successful_transaction_operation_count and failed_transaction_operation_count?

@tomerweller
Copy link
Contributor Author

tomerweller commented Jun 11, 2019 •
edited
Loading

@bartekn that sounds even more confusing to me :)
Maybe failure is a strong word. How about applied_operation_count and not_applied_operation_count ?

@ire-and-curses
Copy link
Contributor

+1 for "applied"

@bartekn
Copy link
Contributor

bartekn commented Jun 12, 2019

I think applied is also confusing because of stellar-core transaction life cycle terminology:

Once SCP agrees on a particular transaction set, that set is applied to the ledger.

But maybe it's because I've been working with stellar-core related code recently?

What do you think about: operation_count_in_successful_transactions and operation_count_in_failed_transactions? 😉

@bartekn bartekn added this to the Horizon 0.19.0 milestone Jun 13, 2019
@tomerweller
Copy link
Contributor Author

That's a lot of words @bartekn, though passable. Going to throw one last proposal in the mix.
I'm rethinking the capacity motivation and to measure that what we actually need is just the number of operations in the txset. So maybe add a txset_operation_count and leave the current operation_count as is is?

@bartekn
Copy link
Contributor

bartekn commented Jun 13, 2019

Sounds good to me. I added it to Horizon 0.18.1 milestone so we should probably start working on this to be able to release it in this sprint.

bartekn added a commit that referenced this issue Jun 18, 2019
@bartekn
services/horizon: Fix ledger capacity usage in fee stats (#1386)
f570483 
In protocol version 11 that implemented CAP-0005 the number of
operations is used as the indicator of ledger capacity. /fee_stats
endpoint is incorrectly using transaction count for ledger capacity
stats.

The query that calculates ledger capacity in the last 5 ledgers has been
rewritten. Because we don't have access to number of failed operations
in history_ledgers table (#1385) we first run subquery that returns
number of operations for ledgers in question along with max_tx_set_size
for each ledger and then calculate the capacity in the parent query.
@ire-and-curses ire-and-curses added the horizon-api Issues or features related to the Horizon API label Jul 19, 2019
@bartekn bartekn removed this from the Horizon 0.18.1 milestone Jul 24, 2019
@tomerweller tomerweller mentioned this issue Jun 2, 2020
Open
@ire-and-curses ire-and-curses added this to the Horizon 1.5.0 milestone Jun 3, 2020
@abuiles abuiles self-assigned this Jun 12, 2020
@abuiles abuiles mentioned this issue Jun 12, 2020
Merged
7 tasks
abuiles added a commit that referenced this issue Jun 16, 2020
@abuiles
services/horizon: Add transaction set operation count to history_ledg…
dce848d 
…er. (#2690)

### What

Extend ingestion to store the total number of operations in the transaction set and expose it in the ledger resource via `tx_set_operation_count`.

### Why

This feature was requested in #1385 and it will help us assess the used capacity of a transaction set.

With this change we have now the total count of operations in the transaction set and the number of successful operations.

Fix #1385.
@abuiles
Copy link
Contributor

abuiles commented Jun 16, 2020

Fixed in #2690 - this will be in the next horizon release.

@abuiles abuiles closed this as completed Jun 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@abuiles abuiles

Labels
horizon horizon-api Issues or features related to the Horizon API
Projects
None yet
Milestone
Horizon 1.5.0
Development

Successfully merging a pull request may close this issue.

services/horizon: Add transaction set operation count to history_ledger. abuiles/go
4 participants
@abuiles @bartekn @tomerweller @ire-and-curses