Re: Terminating provider process

Jan Safranek <[email protected]> Tue, 28 May 2013 11:18:36 +0200
Newsgroups gmane.network.open-pegasus.general
Message-ID <[email protected]>
On 05/17/2013 07:10 PM, Devchandra L Meetei wrote:
> Hi Jan
> As Marek pointed out, We have this in our radar. Please look at bug 9657
> <http://bugzilla.openpegasus.org/show_bug.cgi?id=9657> which will track
> this. 

I agree that these 5 minutes should be configurable. But my problem is
different - Pegasus unloads provider after 5 minutes, but destroys the
provider process in *another* 5 minutes. I need to shut down the process
immediately when the provider is unloaded.

I finally dived into sources, luckily ProviderAgent looks simple enough.
Its main loop is in ProviderAgent::run(), which can be simplified into:

ProviderAgent::run()
{
	while (!_terminating) {
		active = _readAndProcessRequest();
		if (!active &&
			!_providerManagerRouter.hasActiveProviders() &&
			(_threadPool.runningCount() == 0))
		_terminating = true
	}
}


ProviderAgent::readAndProcessRequest()
{
	// process other messages
	if (wakeup_message) {
		_unloadIdleProviders();
		return false
	}
	return true;
}

and finally

ProviderAgent::_unloadIdleProviders()
{
	// start a thread to unload idle providers
}

So, the first wake up message just starts a thread to unload idle
providers, which depends on provider speed and may take some time.
Therefore the main run() loop continues processing next requests.
Only after the second wake up message run() recognizes, that all
providers are already unloaded and exits.

Attached highly experimental patch reworks _unloadIdleProviders() into:

ProviderAgent::_unloadIdleProviders()
{
	// start a thread to unload idle providers
	// wait for the thread to finish
}

Thus when readAndProcessRequest() returns false, all providers are
already unloaded, run() checks it and exits the main loop. All this
happens after the *first* wake up message.


I hope I did not miss any error case or so.

In addition, the patch waits up to
PEGASUS_DEFAULT_SHUTDOWN_TIMEOUT_SECONDS_STRING (=30) seconds for the
providers to unload and the ProviderAgent cane get blocked for this time
if a provider takes too much time to unload. The providers were idle for
5 minutes, so let's hope they can wait for another 30 seconds (in the
worst case). It would be nice if this timeout was configurable, I am not
sure how to do it.

> Also was little alarmed by the figure you gave(15 mint), I was under the
> impression that it is 5 minute(unless modified at packaging).

You are right, it's 5 minutes, not 15.

Jan
test.patch (text/x-patch, 1018 B)
diff -up pegasus/src/Pegasus/ProviderManagerService/ProviderAgent/ProviderAgent.cpp.test pegasus/src/Pegasus/ProviderManagerService/ProviderAgent/ProviderAgent.cpp
--- pegasus/src/Pegasus/ProviderManagerService/ProviderAgent/ProviderAgent.cpp.test	2011-08-11 14:12:36.000000000 +0200
+++ pegasus/src/Pegasus/ProviderManagerService/ProviderAgent/ProviderAgent.cpp	2013-05-28 09:50:56.639410167 +0200
@@ -923,6 +928,19 @@ void ProviderAgent::_unloadIdleProviders
          PEG_TRACE_CSTRING(TRC_PROVIDERAGENT, Tracer::LEVEL1,
              "Could not allocate thread to unload idle providers.");
     }
+
+    // Wait for the cleanup thread to finish
+    Uint64 shutdownTimeout = 0;
+    StringConversion::stringToUnsignedInteger(
+        PEGASUS_DEFAULT_SHUTDOWN_TIMEOUT_SECONDS_STRING,
+        shutdownTimeout);
+
+    while ((_unloadIdleProvidersBusy.get() > 0) && (shutdownTimeout > 0)) {
+        Threads::yield();
+        Threads::sleep(1000);
+        shutdownTimeout--;
+    }
+
     PEG_METHOD_EXIT();
 }