-
Notifications
You must be signed in to change notification settings - Fork 38
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
Fix/put on busy shards #2965
base: master
Are you sure you want to change the base?
Fix/put on busy shards #2965
Conversation
0f8b19e
to
03f1f87
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2965 +/- ##
==========================================
- Coverage 23.47% 23.47% -0.01%
==========================================
Files 780 780
Lines 46655 46688 +33
==========================================
+ Hits 10953 10958 +5
- Misses 34833 34859 +26
- Partials 869 871 +2 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Pavel Karpy <[email protected]>
03f1f87
to
13b546f
Compare
If every shard's pool is overloaded with routines, choose the best one and try to PUT an object to it 30 seconds. Closes #2871. Signed-off-by: Pavel Karpy <[email protected]>
13b546f
to
12246c2
Compare
Well, it works. There is not much we can do about testing currently. It is hard to test in general because it is time-dependent. But it is quite straightforward code, and the tests are okay. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's an improvement, but channels/routines would be even better.
} | ||
|
||
func (e *StorageEngine) putToShardWithDeadLine(sh hashedShard, ind int, pool util.WorkerPool, addr oid.Address, prm PutPrm) error { | ||
const deadline = 30 * time.Second |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make it configurable? Treat zero as old behavior?
err = errPutShard | ||
err = e.putToShardWithDeadLine(bestShard, 0, bestPool, addr, prm) | ||
if err != nil { | ||
e.log.Warn("last stand to put object to the best shard", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't deserve a warn
to me.
@@ -167,19 +214,20 @@ func (e *StorageEngine) putToShard(sh hashedShard, ind int, pool util.WorkerPool | |||
return | |||
} | |||
|
|||
e.reportShardError(sh, "could not put object to shard", err) | |||
e.reportShardError(sh, "could not put object to shard", errGlobal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be left as is?
defer timer.Stop() | ||
|
||
const putCooldown = 100 * time.Millisecond | ||
ticker := time.NewTicker(putCooldown) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One of the reasons I don't like pools we have now. Channels provide natural queueing and blocking/unblocking.
@roman-khimov, what do you mean by that? Sending tasks via channels to every shard? What is the difference? |
How can I get a "block for 30s" behavior with a pool? How can I get a queue with a pool? That's the difference. |
|
No description provided.