Skip to content

Fix how enconding_ns is calculated.#3692

Merged
martinmr merged 1 commit intomasterfrom
martinmr/fix-encoding-ns-calculation
Jul 19, 2019
Merged

Fix how enconding_ns is calculated.#3692
martinmr merged 1 commit intomasterfrom
martinmr/fix-encoding-ns-calculation

Conversation

@martinmr
Copy link
Copy Markdown
Contributor

@martinmr martinmr commented Jul 19, 2019

Currently, the time taken to receive the query response is included as
part of the encoding time. This PR fixes that calculation.


This change is Reviewable

Currently, the time taken to receive the query response is included as
part of the encoding time. This PR fixes that calculation.
@martinmr martinmr requested a review from campoy July 19, 2019 19:06
@martinmr martinmr requested a review from manishrjain as a code owner July 19, 2019 19:06
@martinmr martinmr requested a review from a team July 19, 2019 19:06
Copy link
Copy Markdown
Contributor

@campoy campoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 3 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @manishrjain)

@gitlw
Copy link
Copy Markdown

gitlw commented Jul 19, 2019

Can you please define what the l.Transport means and the logic behind this equation?
l.Transport = time.Since(l.Start) - l.Parsing - l.Processing

@martinmr
Copy link
Copy Markdown
Contributor Author

I'll add a comment in the next PR to avoid having to run teamcity tests again, but transport is the time taken from the completion of the query to receiving the response (sometimes the query is processed by a different alpha). We are doing this because while debugging another issue, we noticed that the encoding time contains the time to receive the response.

@martinmr martinmr merged commit 06c94ba into master Jul 19, 2019
@martinmr martinmr deleted the martinmr/fix-encoding-ns-calculation branch July 19, 2019 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants