eclipse-ee4j / glassfish

Eclipse GlassFish
https://eclipse-ee4j.github.io/glassfish/
386 stars 144 forks source link

setting logger level should not report that server requires restart #15541

Closed glassfishrobot closed 13 years ago

glassfishrobot commented 13 years ago

Using CLI to do set-log-levels, server required restart and gave the wrong reason. The log level doesn't change and indeed require server restart. This is a regression from v2 since v2 log level change is dynamic.

1. using CLI

I change the log level for org.glassfish.admingui from INFO to FINEST. The log level is changed in logging.properties file. GUI is still logged in INFO level. domain required restart and gave the wrong reason.

~/Awork/V3/v3 39) v3admin list-domains domain1 running Command list-domains executed successfully. ~/Awork/V3/v3 40) ~/Awork/V3/v3 40) ~/Awork/V3/v3 40) v3admin set-log-levels org.glassfish.admingui=FINEST org.glassfish.admingui package set with log level FINEST. These logging levels are set for server. Command set-log-levels executed successfully. ~/Awork/V3/v3 41) ~/Awork/V3/v3 41) ~/Awork/V3/v3 41) v3admin list-domains domain1 running, restart required to apply configuration changes domainTEST-secure not running Command list-domains executed successfully. ~/Awork/V3/v3 42) ~/Awork/V3/v3 42) v3admin _get-restart-required --why true server log filename changed. Command _get-restart-required executed successfully. ~/Awork/V3/v3 43)

If i restart the server, GUI starts logging at FINEST.

=================================

Now, when i use GUI to change the log level, I am seeing something different, although GUI is still calling the set-log-levels command. It still says server requires restart, and give the exact reason, "server log filename changed" which is wrong. However, i see that GUI logs at FINEST immediately. But then, there will be exception shown when changing the log level again. The exception says:

[#|2011-01-11T20:05:51.459-0800|SEVERE|glassfish3.1|null|_ThreadID=28;_ThreadName=admin-thread-pool-4848(7);|Cannot read logging.properties file : java.io.IOException: Bad file descriptor at java.io.FileInputStream.close0(Native Method) at java.io.FileInputStream.close(FileInputStream.java:259) at com.sun.common.util.logging.LoggingConfigImpl.openPropFile(LoggingConfigImpl.java:112) at com.sun.common.util.logging.LoggingConfigImpl.updateLoggingProperties(LoggingConfigImpl.java:274) at com.sun.enterprise.server.logging.commands.SetLogLevel.execute(SetLogLevel.java:191) at com.sun.enterprise.v3.admin.CommandRunnerImpl$1.execute(CommandRunnerImpl.java:354) at com.sun.enterprise.v3.admin.CommandRunnerImpl.doCommand(CommandRunnerImpl.java:369) at com.sun.enterprise.v3.admin.CommandRunnerImpl.doCommand(CommandRunnerImpl.java:1080) at com.sun.enterprise.v3.admin.CommandRunnerImpl.access$1200(CommandRunnerImpl.java:95) at com.sun.enterprise.v3.admin.CommandRunnerImpl$ExecutionContext.execute(CommandRunnerImpl.java:1260) at com.sun.enterprise.v3.admin.CommandRunnerImpl$ExecutionContext.execute(CommandRunnerImpl.java:1248) at org.glassfish.admin.rest.ResourceUtil.runCommand(ResourceUtil.java:202) at org.glassfish.admin.rest.resources.TemplateExecCommand.executeCommand(TemplateExecCommand.java:127) at org.glassfish.admin.rest.resources.TemplateCommandPostResource.processPost(TemplateCommandPostResource.java:81) at sun.reflect.GeneratedMethodAccessor211.invoke(Unknown Source)

======================= Please fix it so that set-log-levels does NOT require server restart whether it is using CLI or GUI.

Environment

all platform

Affected Versions

[3.1_dev]

glassfishrobot commented 6 years ago
glassfishrobot commented 13 years ago

@glassfishrobot Commented anilam said: I also see this exception when try to save log level again.

[#|2011-01-11T20:42:54.245-0800|SEVERE|glassfish3.1|null|_ThreadID=31;_ThreadName=pool-26-thread-1;|Cannot read logging.properties file : java.io.IOException: Bad file descriptor at java.io.FileInputStream.readBytes(Native Method) at java.io.FileInputStream.read(FileInputStream.java:177) at java.util.Properties$LineReader.readLine(Properties.java:487) at java.util.Properties.load0(Properties.java:337) at java.util.Properties.load(Properties.java:325) at com.sun.common.util.logging.LoggingConfigImpl.openPropFile(LoggingConfigImpl.java:111) at com.sun.common.util.logging.LoggingConfigImpl.getLoggingProperties(LoggingConfigImpl.java:380) at com.sun.enterprise.server.logging.LogManagerService.getLoggingProperties(LogManagerService.java:124) at com.sun.enterprise.server.logging.LogManagerService$1.changed(LogManagerService.java:315) at org.glassfish.kernel.FileMonitoringImpl$3.run(FileMonitoringImpl.java:124) at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:441) at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:303) at java.util.concurrent.FutureTask.run(FutureTask.java:138) at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908) at java.lang.Thread.run(Thread.java:680)

| #] |

glassfishrobot commented 13 years ago

@glassfishrobot Commented naman_mehta said: As per your comment,

Using CLI,

1. There is no restart required at all. 2. I tested same by changing CORE_LOGGER to INFO to FINEST to INFO. It works fine. 3. Still I see the reason it's showing about 'server logfile name changed'. So just messages is wrong/not needed but functionality works fine without any restart.

Using GUI, 1. Restart required. 2. I tested same by changing CORE_LOGGER to INFO to FINEST to INFO. It's not logging fine messages. 3. Still I see the reason it's showing about 'server logfile name changed'. So just messages is wrong/not needed but functionality works fine without any restart. 4. Not find any exception server.log file.

So there is some issue the way GUI is calling set-log-level command. When I run command using CLI it's not needed any restart.

I need to fix message in CLI about restart required. I was not aware of the same before this bug raised.

glassfishrobot commented 13 years ago

@glassfishrobot Commented anilam said: I think we are seeing the exact opposite, maybe it depends on which logger we are changing the log level ?

Using CLI, I see that I indeed need to restart the server before i see the log level change, but you are saying that you don't need to restart server. It is only the error in the notification.

Using GUI, I see that the log level changed immediately, but you are seeing the opposite.

I will request QA to try the same and see what they get.

glassfishrobot commented 13 years ago

@glassfishrobot Commented naman_mehta said: I fixed restart part and the message in LogManagerService.java.

So using CLI, 1. Now everything works fine. No error in server.log. 2. No message for restart. 3. Changes in log level works without any restart.

Using GUI, 1. Now everything works fine. No error in server.log. 2. No message for restart. 3. Changes in log level works without any restart.

glassfishrobot commented 13 years ago

@glassfishrobot Commented naman_mehta said: I tested same using CLI and GUI. I tried to change all log levels to INFO to FINEST to INFO and works fine. No need of restart.

I am not getting any exception in server.log so that is out of scope for fixing this issue. I just fixed the restart required part in this bug.

glassfishrobot commented 13 years ago

@glassfishrobot Commented naman_mehta said: How bad is its impact? (Severity)

How often does it happen? (Frequency)

How much effort is required to fix it? (Cost)

What is the risk of fixing it? (Risk)

Does a work around for the issue exist? Can the workaround be reasonably employed by the end user?

If the issue is not fixed should the issue and its workaround (if applicable) be described in the Release Notes?

Index: src/main/java/com/sun/enterprise/server/logging/LogManagerService.java

— src/main/java/com/sun/enterprise/server/logging/LogManagerService.java (revision 44384) +++ src/main/java/com/sun/enterprise/server/logging/LogManagerService.java (working copy) @@ -338,7 +338,7 @@

} else if (a.endsWith(".file")) { //check if file name was changed and send notification

glassfishrobot commented 13 years ago

@glassfishrobot Commented anilam said: Approved for 3.1

glassfishrobot commented 13 years ago

@glassfishrobot Commented naman_mehta said: Committed revision 44415.

glassfishrobot commented 13 years ago

@glassfishrobot Commented Issue-Links: is duplicated by GLASSFISH-15488

glassfishrobot commented 13 years ago

@glassfishrobot Commented Was assigned to naman_mehta

glassfishrobot commented 7 years ago

@glassfishrobot Commented This issue was imported from java.net JIRA GLASSFISH-15541

glassfishrobot commented 13 years ago

@glassfishrobot Commented Reported by anilam

glassfishrobot commented 13 years ago

@glassfishrobot Commented Marked as fixed on Tuesday, January 11th 2011, 2:53:40 pm