MGO-160 Set tailable cursor dealine before waiting on any getMores #20
Conversation
session.go
Outdated
| // we should expect more data. | ||
| // either there is a getMore running or we will schedule one, set | ||
| // the deadline regardless. | ||
| if deadline.IsZero() { |
There was a problem hiding this comment.
There's subtle interplay between having deadline and having iter.timeout. I suggest protecting this if block with deadline.IsZero && iter.timeout >= 0, then replace line 3842 with if ! deadline.IsZero.
| // we should expect more data. | ||
| // either there is a getMore running or we will schedule one, set | ||
| // the deadline regardless. | ||
| if iter.timeout >= 0 && deadline.IsZero() { |
There was a problem hiding this comment.
Super minor nitpick, but assuming iter.timeout is usually true and deadline.IsZero is usually false (after the first time through the loop), then reversing the predicate order will shortcut faster.
There was a problem hiding this comment.
But then it will shortcut slower for the more general case where there is no timeout at all.
There was a problem hiding this comment.
I'd assume a timeout is more common, but it doesn't really matter in the real world.
There was a problem hiding this comment.
I believe timeouts are only set for tailable cursors that's why I think it would be the minority.
There was a problem hiding this comment.
I'm relying on Travis for testing: https://travis-ci.org/10gen/mgo/builds/452479168
Do we test mgo in Evergreen?
Still trying to determine how to reliably trigger this case in a test. Working on it here: https://github.com/ShaneHarvey/mgo/commits/fix-tailable-timeout-test
| // we should expect more data. | ||
| // either there is a getMore running or we will schedule one, set | ||
| // the deadline regardless. | ||
| if iter.timeout >= 0 && deadline.IsZero() { |
There was a problem hiding this comment.
But then it will shortcut slower for the more general case where there is no timeout at all.
|
Travis uses a very old version of mongod. |
|
I added 3.2-4.0 testing to travis: ShaneHarvey@5694f97 and tested in https://travis-ci.org/ShaneHarvey/mgo/builds/452550408. 3.4 and 4.0 fail for strange reasons that I think are unrelated to the timeout change. This is the build: https://travis-ci.org/ShaneHarvey/mgo/builds/452550408 I think we should investigate these issues in: #21 |
Jira: https://jira.mongodb.org/browse/MGO-160