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

Add timeout to sequencing context #1595

Merged
merged 3 commits into from
May 14, 2019
Merged

Conversation

AlCutter
Copy link
Member

Checklist

server/log_operation_manager.go Outdated Show resolved Hide resolved
server/log_operation_manager.go Outdated Show resolved Hide resolved
@AlCutter AlCutter force-pushed the deadline_signer branch 4 times, most recently from f14ce28 to 1c38c29 Compare May 13, 2019 17:38
@AlCutter AlCutter requested a review from RJPercival May 13, 2019 17:43
@codecov
Copy link

codecov bot commented May 13, 2019

Codecov Report

Merging #1595 into master will decrease coverage by <.01%.
The diff coverage is 61.53%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1595      +/-   ##
==========================================
- Coverage   67.18%   67.17%   -0.01%     
==========================================
  Files         110      110              
  Lines        8968     8976       +8     
==========================================
+ Hits         6025     6030       +5     
- Misses       2338     2339       +1     
- Partials      605      607       +2
Impacted Files Coverage Δ
server/log_operation_manager.go 78.6% <61.53%> (-0.63%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2da1f71...e59fbfd. Read the comment docs.

@codecov
Copy link

codecov bot commented May 13, 2019

Codecov Report

Merging #1595 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1595      +/-   ##
==========================================
+ Coverage   67.18%   67.19%   +0.01%     
==========================================
  Files         110      110              
  Lines        8968     8972       +4     
==========================================
+ Hits         6025     6029       +4     
  Misses       2338     2338              
  Partials      605      605
Impacted Files Coverage Δ
server/log_operation_manager.go 79.62% <100%> (+0.39%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2da1f71...c87a765. Read the comment docs.

@AlCutter AlCutter requested review from pav-kv and removed request for RJPercival May 13, 2019 18:16
@AlCutter AlCutter assigned pav-kv and unassigned RJPercival May 13, 2019
@pav-kv
Copy link
Contributor

pav-kv commented May 13, 2019

If the timeout triggers, how good/informative is the output in glog? Will it be clear that sequencing runs are timing out?

CHANGELOG.md Outdated

#### Add timeout to sequencing loop

Added a timeout to the context in the sequencing loop.
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be helpful to mention it's a 60 second timeout.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@AlCutter
Copy link
Member Author

If the timeout triggers, how good/informative is the output in glog? Will it be clear that sequencing runs are timing out?

Yup, you'll see something along the lines of: <error doing something>: context deadline exceeded

@AlCutter AlCutter merged commit 3e9002c into google:master May 14, 2019
@AlCutter AlCutter deleted the deadline_signer branch May 14, 2019 16:36
gdbelvin added a commit that referenced this pull request May 17, 2019
* master: (54 commits)
  Couple of changes to make NodeIDs more frugal. (#1613)
  compact.Tree: Change semantic of adding leaves (#1592)
  MapHasher: Do not return error from HashLeaf (#1611)
  Move Postgres schema into "schema" subdirectory
  Move MySQL schema into "schema" subdirectory
  Improve FlipRightBit by doing it directly. (#1610)
  log_subtree_cache: Use compact Range instead of Tree (#1609)
  LogHasher: Return no error from HashLeaf method (#1608)
  Remove update_changelog.sh
  Fewer copies of the bytes from big.Int (#1602)
  Switch some low-level logging in hashing related code to 'if' guards (#1601)
  Merge pull request #1596 from RJPercival/del_dockerfile_db
  Correct name of "licenses" binary in .gitignore
  compact.Tree: Allow nil visitor and add benchmarks (#1599)
  Add timeout to sequencing context (#1595)
  Combine `go install` commands in .travis.yml
  Combine `go get` commands in .travis.yml
  Sort `go get` commands in .travis.yml
  Rename "indirect/external.go" to "indirect/indirect.go"
  Fix typo: "TestLibaries" -> "TestLibraries"
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants