Cancel writecache background flush as soon as possible #1205

dstepanov-yadro · 2024-06-26T08:40:36Z

dstepanov-yadro commented

2024-06-26 08:40:36 +00:00

Scenario:

Writecache is full and flushes big objects
frostfs-node tries to close writecache

Writecache will try to get modeMtx lock to cancel background flush, but lock is taken by big object flush worker.

Closes #1201

Scenario: 1. Writecache is full and flushes `big` objects 2. `frostfs-node` tries to close writecache Writecache will try to get `modeMtx` lock to cancel background flush, but lock is taken by big object flush worker. Closes #1201

dstepanov-yadro requested review from storage-core-committers 2024-06-26 09:24:48 +00:00

dstepanov-yadro requested review from storage-core-developers 2024-06-26 09:24:49 +00:00

acid-ant approved these changes 2024-06-26 09:53:00 +00:00

aarifullin approved these changes 2024-06-26 10:52:46 +00:00

achuprov approved these changes 2024-06-26 10:57:37 +00:00

fyrchik requested changes 2024-06-26 11:04:23 +00:00

pkg/local_object_storage/writecache/flush.go Outdated

					
				@ -151,3 +151,3 @@

				func (c *cache) workerFlushBig(ctx context.Context) {

					tick := time.NewTicker(defaultFlushInterval * 10)

					tick := time.NewTicker(defaultFlushInterval)

fyrchik commented

2024-06-26 11:04:21 +00:00

Please, describe the motivation behind this change in the commit message.
It can affect perfomance, as we do not currently remove empty directories automatically.

Please, describe the motivation behind this change in the commit message. It can affect perfomance, as we do not currently remove empty directories automatically.

dstepanov-yadro commented

2024-06-26 11:13:12 +00:00

Done.
It looks like it's some kind of debug code. This code appeared in the very first cache implementation without any explanation.

Done. It looks like it's some kind of debug code. This code appeared in the very first cache implementation without any explanation.

fyrchik commented

2024-06-27 13:21:39 +00:00

It was actually intentional, because we should have less big objects and more small ones.
Iterating over the DB is much cheaper that touching filesystem.
I would introduce a separate constant for big iteration, as the magic constant is indeed the problem.

It was actually intentional, because we should have less big objects and more small ones. Iterating over the DB is much cheaper that touching filesystem. I would introduce a separate constant for big iteration, as the magic constant is indeed the problem.

dstepanov-yadro commented

2024-06-27 14:06:00 +00:00

Ok, dropped commit with this change.