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

Bug: nonsensical print of Ticket processing #2961

Open
stronk-dev opened this issue Jan 29, 2024 · 8 comments
Open

Bug: nonsensical print of Ticket processing #2961

stronk-dev opened this issue Jan 29, 2024 · 8 comments
Labels
status: triage this issue has not been evaluated yet

Comments

@stronk-dev
Copy link
Contributor

As of PR #2907, logging of received tickets have been changed.
It now sums the FV, win percentage and EV and prints only the sum, which make these variables useless: the parameters of a game of chance do not change when doing multiple runs. Specifically, it breaks my panels as without knowing how many tickets were added, I cannot derive any useable FV, win% or EV.

Since summing these values is nonsenical, can we roll back the PR or modify it to print an average?

https://github.com/ad-astra-video/go-livepeer/blob/7fdbde80aa075b963f8210958a69cb93093347c9/core/orchestrator.go#L219

@github-actions github-actions bot added the status: triage this issue has not been evaluated yet label Jan 29, 2024
@stronk-dev
Copy link
Contributor Author

stronk-dev commented Jan 29, 2024

Never mind, looks like i mixed up some repos (and i need to update my binaries). I can't find the commit (7fdbde80aa075b963f8210958a69cb93093347c9) of the PR applied to orchestrator.go

@thomshutt
Copy link
Contributor

😄 the best kind of bug report

@stronk-dev
Copy link
Contributor Author

stronk-dev commented Jan 29, 2024

New checklist before opening a GH issue/PR:

  • Ingest 1 unit of caffeine
  • Do not verify info
  • Include snarky comment in OP

@stronk-dev
Copy link
Contributor Author

stronk-dev commented Jan 30, 2024

Never mind, it looks the commit is in the master branch after all: ddd7e7e

I get that summarizing the EV makes sense, but this is already exposed as a prometheus metric to track it. Having a totalFaceValue or totalWinProb doesn't make sense to me, as you can no longer use them to calculate anything without converting it back to a per-ticket basis

@stronk-dev stronk-dev reopened this Jan 30, 2024
@FranckUltima
Copy link

Since the update to 0.7.2, I am actually encountering the same problem and some of my Grafana dashboards no longer really make sense. :)

@ad-astra-video
Copy link
Collaborator

I can get a PR up for this to change to average and add number of tickets processed to the log line.

I personally do not prefer to get the logs overwhelmed by a log line for each ticket processed with new lower ticketEV.

@stronk-dev, for now, are the panels not able to be adjusted in grafana with the ticketEV known by the O?

@stronk-dev
Copy link
Contributor Author

stronk-dev commented Jan 31, 2024

I can get a PR up for this to change to average and add number of tickets processed to the log line.
I personally do not prefer to get the logs overwhelmed by a log line for each ticket processed with new lower ticketEV.

Since these ticket params are usually constant (correct me if i am wrong on this assumption), we could prevent calculating the average value. You could print something like: W tickets processed with X face value and Y winProb for a total EV of Z. Any time the face value or winprob changes, start a new print.

@stronk-dev, for now, are the panels not able to be adjusted in grafana with the ticketEV known by the O?

Nope, it's Loki log parsing extracting the face value and win percentage specifically. I'm unable to parse this back into a per-ticket basis at the moment to do calculations with

@ad-astra-video
Copy link
Collaborator

The faceValue and winProb are the same for each ticket.

O provides the faceValue and winProb then the B provides a nonce and signs the ticket with the nonce. The B then sends back a list of signatures over the ticket params and the nonce for each ticket to the O.

I think your solution works and side steps additional calculation since we can just pull the faceValue and the winProb from the first ticket in the batch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: triage this issue has not been evaluated yet
Projects
None yet
Development

No branches or pull requests

4 participants