geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From John Blum <jb...@pivotal.io>
Subject Re: [PROPOSAL] Add getCache and getLocator to Launchers
Date Sat, 03 Nov 2018 00:49:39 GMT
Damn it! Correction to my previous email...

'--disable-default-port' should read '--disable-default-server' in my *Gfsh*
`start server` command examples.

Apologies,
John


On Fri, Nov 2, 2018 at 5:41 PM, John Blum <jblum@pivotal.io> wrote:

> Bruce-
>
> Regarding...
>
> > "... *but it's the AcceptorImpl thread that keeps the JVM from exiting,
> so I don't really agree that you can create a server with gfsh that doesn't
> have a CacheServer.*"
>
> Well, that is not entirely true, and the last bit is definitely not true.
>
> How do you suppose I can do this...
>
> gfsh>start server --name=Server1
> gfsh>start server --name=Server2 *--server-port*=51515
> gfsh>start server --name=Server3 *--disable-default-port*
> gfsh>start server --name=Server4 *--disable-default-port*
>
> ... if I could not disable the CacheServer?
>
> Clearly, Server1 starts up with the default CacheServer port, 40404.
> Server2 explicitly sets the CacheServer port to 51515.  And Servers 3 & 4
> simply do not have CacheServers running.  If they tried to start a
> CacheServer, and they did NOT explicitly set the --server-port option,
> then Servers 3 & 4 would result in throwing a java.net.BindException.
>
> So, the ServerLauncher class "blocks" if you do not start a CacheServer
> instance, which would not be started if the server was started using `start
> server --disable-default-server`).
>
> See here [1], then here [2], and then here [3] and here [4] as well as
> this [5], which gets set from this [6].
>
> There is a very good reason why I know this.
>
> Regards,
> -John
>
>
> [1] https://github.com/apache/geode/blob/rel/v1.7.0/geode-
> core/src/main/java/org/apache/geode/distributed/ServerLauncher.java#L705
> [2] https://github.com/apache/geode/blob/rel/v1.7.0/geode-
> core/src/main/java/org/apache/geode/distributed/
> ServerLauncher.java#L906-L928
> [3] https://github.com/apache/geode/blob/rel/v1.7.0/geode-
> core/src/main/java/org/apache/geode/distributed/ServerLauncher.java#L953
> [4] https://github.com/apache/geode/blob/rel/v1.7.0/geode-
> core/src/main/java/org/apache/geode/distributed/
> ServerLauncher.java#L940-L942
> [5] https://github.com/apache/geode/blob/rel/v1.7.0/geode-
> core/src/main/java/org/apache/geode/distributed/
> ServerLauncher.java#L407-L409
> [6] https://github.com/apache/geode/blob/rel/v1.7.0/geode-
> core/src/main/java/org/apache/geode/distributed/ServerLauncher.java#L1487
>
>
> On Fri, Nov 2, 2018 at 3:03 PM, Bruce Schuchardt <bschuchardt@pivotal.io>
> wrote:
>
>> Hmm, but it's the AcceptorImpl thread that keeps the JVM from exiting, so
>> I don't really agree that you can create a server with gfsh that doesn't
>> have a CacheServer.  One's got to be created through cache.xml or something.
>>
>> Be that as it may I think the recent talk about this convinces me that
>> Kirk's original proposal is sound and we should go with it. Servers can be
>> fished out of the cache so it's not a big deal if the launcher API doesn't
>> have a getCacheServer method.
>>
>>
>> On 11/1/18 9:38 AM, John Blum wrote:
>>
>>> Well, ServerLauncher may or may not create a CacheServer instance when it
>>> starts a server.  A server can be created without a CacheServer, for
>>> instance, when in *Gfsh* `start server --disable-default-server` option
>>> is
>>>
>>> specified.
>>>
>>> In addition, you can always find or get the list of CacheServers (if
>>> present) from the Cache instance, using
>>> Cache.getCacheServers():List<CacheServer> [1].  So, I think it would be
>>> better if the ServerLauncher returned a handle to the Cache and then
>>> drill
>>> down as opposed to up.
>>>
>>> -j
>>>
>>>
>>> [1]
>>> http://geode.apache.org/releases/latest/javadoc/org/apache/g
>>> eode/cache/Cache.html#getCacheServers--
>>>
>>>
>>> On Thu, Nov 1, 2018 at 7:31 AM, Bruce Schuchardt <bschuchardt@pivotal.io
>>> >
>>> wrote:
>>>
>>> I like this but it might be even better if ServerLauncher gave access to
>>>> the CacheServer it launched and CacheServer gave access to its cache.
>>>> The
>>>> Locator interface, as well, could have a getCache() method and deprecate
>>>> the getDistributedSystem() method.  These days a Locator always has a
>>>> Cache
>>>> and no-one is interested in its DistributedSystem.
>>>>
>>>>
>>>> On 10/31/18 2:48 PM, Kirk Lund wrote:
>>>>
>>>> LocatorLauncher provides an API which can be used in-process to create a
>>>>> Locator. There is no public API on that class to get a reference to the
>>>>> Locator or its Cache.
>>>>>
>>>>> Similarly, ServerLauncher provides an API which can be used in-process
>>>>> to
>>>>> create a Server, but there is no public API in that class to get a
>>>>> reference to its Cache.
>>>>>
>>>>> The User of either Launcher would then have to resort to invoking
>>>>> singletons to get a reference to the Cache.
>>>>>
>>>>> There are existing package-private getter APIs on both Launchers but
>>>>> they're only used by tests in that same package.
>>>>>
>>>>> I propose adding public APIs for getCache to both LocatorLauncher and
>>>>> ServerLauncher as well as adding getLocator to LocatorLauncher. The
>>>>> signatures would look like:
>>>>>
>>>>> /**
>>>>>    * Gets a reference to the Cache that was created by this
>>>>> ServerLauncher.
>>>>>    *
>>>>>    * @return a reference to the Cache
>>>>>    */
>>>>> public org.apache.geode.cache.Cache getCache();
>>>>>
>>>>> /**
>>>>>    * Gets a reference to the Locator that was created by this
>>>>> LocatorLauncher.
>>>>>    *
>>>>>    * @return a reference to the Locator
>>>>>    */
>>>>> public org.apache.geode.distributed.Locator getLocator();
>>>>>
>>>>> Any thoughts? Yay or nay?
>>>>>
>>>>> Thanks,
>>>>> Kirk
>>>>>
>>>>>
>>>>>
>>>
>>
>
>
> --
> -John
> john.blum10101 (skype)
>



-- 
-John
john.blum10101 (skype)

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message