On 13/10/2011 12:32, bugzi...@apache.org wrote:
> https://issues.apache.org/bugzilla/show_bug.cgi?id=46264
>
> --- Comment #19 from Konstantin Kolinko <knst.koli...@gmail.com> 2011-10-13
> 11:32:13 UTC ---
> (In reply to comment #18)
>> Created attachment 27767 [details]
>
> Re: startStopExecutor.allowCoreThreadTimeOut(true);
>
> I think that just using "0" instead of getStartStopThreadsInternal() as the
> value of first argument (corePoolSize) in ThreadPoolExecutor constructor will
> have the same effect. It is not much of a difference though.
I think that is a cleaner approach. I'll make the change.
> Re: Iterator<Future<Void>> iter = results.iterator();
>
> It could be rewritten as for(Future<Void> future: results) loop.
Done.
> In one place Future<?> is used, while I think it could be Future<Void> like in
> other places.
That is the difference between a callable and runnable and I think it is
correct. At least I see errors if I try using Future<Void> there.
> Re: HostConfig
>
> I do not quite understand why to remove
> "if (deploymentExists(cn.getName())) { return; }"
> from the beginning of e.g. deployDescriptor() method.
>
> The HostConfig#deployApps() method is called every 10 seconds to perform
> autodeployment (by HostConfig#check() called by HostConfig#lifecycleEvent())
> and without early return it will proceed to parsing context.xml file.
It wasn't removed, it was moved to the deployDescriptors() method to
prevent a thread being spawned every 10s just to call deploymentExists()
and then return.
I missed the other route to calling that method.
deployDescriptor()
deployApps(String)
check(String)
I'll add a call to deploymentExists() to deployApps(String)
> Renaming s/dir/war/ can be done now, to slightly reduce future patch.
Yep. I'll do that too.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
