Closed abudnik closed 9 years ago
I do not really think this patch is needed - why those people can not use periodic/timeout automatic defragmentation instead?
Client-requested defragmentation was specially introduced this way - it always sorts blobs, because it is needed for recovery. Now, if this flag was set, defragmentation will silently complete while blobs are not actually sorted, and recovery will take forever to complete.
Very bad idea.
There is a service where average lifetime of each record is known to be very short. At the time of defragmentation it has several heavily defragmented blobs that can be easily compacted and merged. Records from the rest blobs will be removed in the nearest future, so it is useless and time-wasting to sort them at the moment.
Then setup periodic or other automatic defrafmentation policy with high defrag percentage option, it was designed to address this kind of constraints.
If this patch is applied, it will not be possible to recover data because of random IO in unsorted blobs. This new flag basically forbids full sorting needed for recovery.
Your case can be solved with existing mechanics, there is no need to introduce this new logic which does break things for nothing.
Then setup periodic or other automatic defrafmentation policy with high defrag percentage option, it was designed to address this kind of constraints.
It doesn't turn off sorting unsorted blobs which are not satisfy defrag percentage. Again in our case we do not need such sorting and for us it is time-wasting, useless pagecache flushing and useless IO activity.
If this patch is applied, it will not be possible to recover data because of random IO in unsorted blobs. >This new flag basically forbids full sorting needed for recovery.
If you don't need this logic - don't use it, that's why it was introduced with new flag.
Your case can be solved with existing mechanics, there is no need to introduce this new logic which does break things for nothing.
Existing mechanics make additional useless action which we want to escape.
It doesn't turn off sorting unsorted blobs which are not satisfy defrag percentage. Again in our case we do not need such sorting and for us it is time-wasting, useless pagecache flushing and useless IO activity.
This is a completely different area - automatic sorting does honor defrag percentage and does not sort blobs which are not heavily fragmented in your notation.
But manual sorting (i.e. what is started with dnet_ioclient) forces all unsorted blobs to be sorted. It was made deliberately - admin asks system to sort blobs, he knows better what to do. Your change breaks that.
It is not about what I will or will not use, it is about design of the system and possible outcomes. Your change breaks design decision made for purpose - your change will kill recovery performance. If you do not see this, and do not think someone will face this problem, it does not mean there is no problem.
This is a problem. You can not introduce flags which may kill disks with random IO just because in short-term range you do not expect to run recovery there.
What we are talking here about? This change introduces a case, where recovery will not work at all. It introduces a flag, which says 'Elliptics distributed storage will not recover data under this conditions'. I do not accept this pull request because of that.
And as a side note, you can achieve your goals with existing methods.
If you want manual control of the defragmentation, I would recommend adding flag into defrag command, not into config file. That defrag command issued by the client would force sorting only eblobs with high enough defrag percentage. It does not replace full defragmentation command which exists now (which is required to run recovery), but extends it.
If you want manual control of the defragmentation, I would recommend adding flag into defrag command, not into config file. That defrag command issued by the client would force sorting only eblobs with high enough defrag percentage. It does not replace full defragmentation command which exists now (which is required to run recovery), but extends it.
Moving this logic to defragmentation command flag is ok and it is what we want to, but it breaks protocol and API.
This is a completely different area - automatic sorting does honor defrag percentage and does not sort blobs which are not heavily fragmented in your notation.
It is another weird behaviour of eblob, why defragmentation started by client (without any flags) has different logic than defragmentation started by eblob itself (without any flags)? It is better to make common flags which can be used at both client and eblob, and will lead to common defragmentation logic.
It doesn't need to break things - dnet_backend_control has plenty of unused space and you can easily introduce new command and API helpers. I added 'set_delay()' method this way.
It looks that you only need to add a new command even without changing dnet_backend_control. That new command will be propagated down to eblob_backend.c where 'want_defrag' will be set and this current patch starts working (config flag is not needed).
Eblob has to export new method like eblob_start_extended_defrag, which will accept external flag field. And that's it, no major changes, no broken API, no cases where recovery suddenly kills the disks.
We can change dnet_backend_callbacks even though it is public, since there are no out-of-the-source-tree elliptics servers which might use this structure
It is another weird behaviour of eblob, why defragmentation started by client (without any flags) has different logic than defragmentation started by eblob itself (without any flags)? It is better to make common flags which can be used at both client and eblob, and will lead to common defragmentation logic.
Defragmentation started by client was deliberately made to force eblob to sort everything unsorted (it will not sort blobs again though, and will not defrag sorted blobs, if defrag percentage is not reached), since it is required by recovery process, which is also started by client.
Client-requested sorting previously was exactly like eblob's internal sorting logic - this ended up with cases when client started sorting, waited for completion, started recovery and it never ended at all because of extremely low random IO performance. There were timeouts usually.
Thus client requested sorting was changed and became an order to sort everything unsorted.
Yes, but it still weird. With common defragmentation flags there will be no hidden logic and if client wants to force sorting all unsorted blobs, he will run defragmentation with corresponding flags and will be sure that eblob will make exactly he requested.
Yes, it is kind of weird and it would be great to have single place for all that logic, but we still need 2 configurations (or commands, or logics): one sorting/defragmentation logic for common tasks (defrag percentage and so on) and another one for recovery (sort everything unsorted).
Historically the latter is called defragmentation and the former will be soft defragmentation or 'heavily fragmented blobs only' defragmentation. While more logically the later is 'sort everything' defragmentation and the former is just common sense defragmentation.
This PR adds special defragmentation mode, which is usefull to compact only heavy-fragmented blobs. Currently some workload consists of constant append and remove operations, so first blobs have lots of removed data. Sysadmins want to free disk space occupied by this sparse blobs, but they don't want to make defragmentation of all blobs, because it pollutes page cache, does many disk i/o for a long time and so on.