Skip to content
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

Unhelpful warning: Warning, unable to set nice value #125

Closed
ole-tange opened this issue Oct 20, 2019 · 7 comments
Closed

Unhelpful warning: Warning, unable to set nice value #125

ole-tange opened this issue Oct 20, 2019 · 7 comments

Comments

@ole-tange
Copy link

When nicing lrz you get the following unhelpful warning:

$ echo | nice nice lrz | wc
Warning, unable to set nice value
Warning, unable to set nice value on thread
Warning, unable to set nice value on thread
      3       2      70

According to top lrz is niced as expected, so the warning is only confusing.

@pete4abw
Copy link
Contributor

Nice is set internally in the program via -N option. There is no test for external use of the nice command. Should there be?

@pete4abw
Copy link
Contributor

pete4abw commented Oct 24, 2019 via email

@ole-tange
Copy link
Author

The situation I discovered this in was a script that was called with nice and which included lrz; so lrz simply inherited the nice value. And I was surprised to see a script that normally worked just fine, now giving warnings.

Most UNIX commands I know will not renice themselves without explicitly being told to. They will let the user decide whether they should be niced or not. I am not sure renicing without telling the user adheres to Principle of Least Surprise.

It seems backwards that the user will have to know lrz quite intimately to have it run at the same nice level as it is called. I have used lrz for several years and even I do not know what options to give lrz to make it run at the same nice level as its parent without giving warnings.

I had hoped lrz could be used as a drop-in replacement for gzip, but this behaviour is different from gzip, and if it is not changed, I hope at least it will be stressed in the man page.

lrz already behaves slightly different from lrzip. Maybe it would make sense to simply remove the renicing when called as lrz?

That solution would fit well with the description from the man page:

lrz is identical to the lrzip application, however, its command line options and behaviour
are made to be as compatible with gzip as possible.

@pete4abw
Copy link
Contributor

BTW the same warning occurs using lrzip or lrzcat. It's not specific to lrz, as expected.

$ echo | nice nice lrzip  | wc
Warning, unable to set nice value
Compression Ratio: inf. Average Compression Speed:  0.000MB/s.
Total time: 00:00:00.00
      3       3       70

However, setting -N ## as a command line option, works fine.

$ echo | lrz -N 19 | wc
      3       3      70

It's an interesting exercise, but since lrzip handles niceness internally, is documented as such, and performs this function as intended, I would say this is not a bug, but a usage error.

@ole-tange
Copy link
Author

ole-tange commented Oct 28, 2019

When I run lrz, it runs at nice level 0. main.c says:

                control->nice_val = 0;

The manual for lrz says: "lrz is identical to the lrzip application, however, its command line options and behaviour are made to be as compatible with gzip as possible."

For that reason I think it would be fair that the setpriority is simply skipped if -N is not set when called as lrz. So:

diff --git a/main.c b/main.c                                                                                                                                                
index f3391a8..de8ba22 100644                                                                                                                                               
--- a/main.c                                                                                                                                                                
+++ b/main.c                                                                                                                                                                
@@ -304,10 +304,11 @@ static void recurse_dirlist(char *indir, char **dirlist, int *entries)                                                                                
                                                                                                                                                                            
 static const char *loptions = "bcCdDefghHiKlL:nN:o:O:p:PqrS:tTUm:vVw:z?";                                                                                                  
 static const char *coptions = "bcCdefghHikKlLnN:o:O:p:PrS:tTUm:vVw:z?123456789";                                                                                           
+static bool compat = false;                                                                                                                                                
                                                                                                                                                                            
 int main(int argc, char *argv[])                                                                                                                                           
 {                                                                                                                                                                          
-   bool lrzcat = false, compat = false, recurse = false;
+ bool lrzcat = false, recurse = false;
        bool options_file = false; /* for environment and tracking of compression setting */
        struct timeval start_time, end_time;
        struct sigaction handler;
@@ -569,8 +570,12 @@ int main(int argc, char *argv[])
                if (unlikely(setpriority(PRIO_PROCESS, 0, control->nice_val / 2) == -1))
                        print_err("Warning, unable to set nice value\n");
        } else {
-           if (unlikely(setpriority(PRIO_PROCESS, 0, control->nice_val) == -1))
+   if(compat && !control->nice_val) {
+     /* Skip */
+   } else {
+     if (unlikely(setpriority(PRIO_PROCESS, 0, control->nice_val) == -1))
                        print_err("Warning, unable to set nice value\n");
+   }
        }
 
        /* One extra iteration for the case of no parameters means we will default to stdin/out */
diff --git a/stream.c b/stream.c
index 4dd2d82..42e7c42 100644
--- a/stream.c
+++ b/stream.c
@@ -91,6 +91,7 @@ static long output_thread;
 static pthread_mutex_t output_lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_cond_t output_cond = PTHREAD_COND_INITIALIZER;
 static pthread_t *threads;
+static bool compat;
 
 bool init_mutex(rzip_control *control, pthread_mutex_t *mutex)
 {
@@ -1276,9 +1277,10 @@ static void *compthread(void *data)
        cti = &cthread[i];
        ctis = cti->sinfo;
 
+ if(compat) {
        if (unlikely(setpriority(PRIO_PROCESS, 0, control->nice_val) == -1))
                print_err("Warning, unable to set nice value on thread\n");
-
+ }
        cti->c_type = CTYPE_NONE;
        cti->c_len = cti->s_len;

@pete4abw
Copy link
Contributor

pete4abw commented Dec 4, 2019

This issue was dealt with in #136 . Issue can be closed.

@ckolivas
Copy link
Owner

Unfortunately #136 did not address this problem, but I will shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants