Project

General

Profile

Feature #6796

ionice support (initially for rdiff-backup)

Added by exobuzz over 3 years ago. Updated over 1 year ago.

Status:
In Progress
Priority:
Normal
Assignee:
-
Category:
rdiff-backup handler
Target version:
-
Start date:
03/01/2014
Due date:
% Done:

20%

QA Check:
Ready for QA

Description

rdiff-backup can cause a lot of IO when scanning though folders. Running with ionice would be useful on top of the nice functionality (I see at least one other has requested it via debian bugs also). Here's a start - if happy I can continue and add it for other handlers that could benefit.

This patch also moves ionice commandline to $execstr so the debug output can show the complete commandline including ionice/nice config - which I think is useful for debugging. it is also only used now if ionice level is set, rather than always with a default of 0

ionice config is done with a "ionice_params" variable so users can manually set the commandline. I was testing with "-c3" for my set up.

ionice.diff View (1.1 KB) exobuzz, 03/01/2014 08:31 PM

0001-Added-ionice_params-and-description-to-example.rdiff.patch View (1.14 KB) ulrich, 03/31/2016 07:15 PM

0002-rdiff.helper.in.patch View (3.25 KB) ulrich, 03/31/2016 08:41 PM

History

#1 Updated by exobuzz over 3 years ago

Was there a reason the "nice" was done before the su before btw that I don't know about ? I would have though better to do in the target root shell, although I guess it will have the same effect ?

#2 Updated by intrigeri over 3 years ago

  • Status changed from New to Confirmed
  • % Done changed from 0 to 10
  • QA Check set to Ready for QA

#3 Updated by intrigeri over 3 years ago

Sorry, I'm unlikely to have time to review this in a timely manner. Anyone else, please don't hesitate giving it a try.

#4 Updated by exobuzz over 3 years ago

It's a simple patch with little changes. I can at least tell you I am running with this change on a few of my machines and all is working fine.

#5 Updated by ulrich over 1 year ago

After I read about the preparations for a new release on the mailing list I decided to review and test some of the proposed patches and report back. First thanks to exobuzz for the proposed patch.

The patch is short an works against the latest git master. I tested it without the ionice_params set and a dozen of well formed and erroneous settings for ionice and all resulted in the expected behavior.

Further more the su -c was moved outside the range of nice. It is needed for ionice but now it is also guaranteed, that a negative nice level is independent of the privileges of the executing script part. For example the oneliner nice -n -5 su -c echo fails as normal user while su -c "nice -n -5 echo" works fine.
At the moment backupninja is always running with root privileges, so it doesn't matter now, but maybe some time in the future. For backups by users or something like that.

#6 Updated by intrigeri over 1 year ago

  • Status changed from Confirmed to In Progress
  • % Done changed from 10 to 20
  • QA Check changed from Ready for QA to Dev Needed

Thanks! Next step is to update examples/example.rdiff so that one can discover this option without having to read the source code (ideally, handlers/rdiff.helper.in should be updated too but I won't block on this).

#7 Updated by ulrich over 1 year ago

Here is a patch for the examples/example.rdiff.
Feel free to enhance the explanation of the options.

I'm going to look into handlers/rdiff.helper.in but need a tester after I finished the patch.

Kind regards,

ulrich

#8 Updated by ulrich over 1 year ago

Here is the promised patch for handlers/rdiff.helper.in.
I added two options for nice and ionice_params to the advanced menu of the configuration wizzard of the rdiff handler. The created config file will contain the two options nice and ionice_params and a short description.
I tested in on my local machine with debian. The patch doesn't add much code to the script and there shouldn't be any problems, but can somebody test it please?

Kind regards,

ulrich

#9 Updated by intrigeri over 1 year ago

  • QA Check changed from Dev Needed to Ready for QA

#10 Updated by ulrich over 1 year ago

My patches can also be found at my git repository (https://github.com/UlrichViefhaus/backupninja) in the branch Feature6796: https://github.com/UlrichViefhaus/backupninja/tree/Feature6796

I hope it helps to integrate the patches in the next release. Can someone test it, please?

Also available in: Atom PDF