[jira] [Closed] (GEODE-5220) DistributedRegion Should Validate AsyncEventQueue/GatewaySender Attributes Earlier

["nabarun (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/GEODE-5220?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

nabarun closed GEODE-5220.
--------------------------

> DistributedRegion Should Validate AsyncEventQueue/GatewaySender Attributes Earlier
> ----------------------------------------------------------------------------------
>
>                 Key: GEODE-5220
>                 URL: https://issues.apache.org/jira/browse/GEODE-5220
>             Project: Geode
>          Issue Type: Bug
>          Components: gfsh, regions, wan
>            Reporter: Juan José Ramos Cassella
>            Assignee: Juan José Ramos Cassella
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 1.7.0
>
>         Attachments: reproducer.zip
>
>          Time Spent: 50m
>  Remaining Estimate: 0h
>
> The method {{DistributedRegion.postCreateRegion()}} currently validates that the {{gateway-sender}} or {{async-event-queue}} associated with the region upon creation (if any) is not of type {{parallel}} as that is not supported by regions of type {{REPLICATED}}:
> {code:title=DistributedRegion.java|borderStyle=solid}
>   @Override
>   public void postCreateRegion() {
>     super.postCreateRegion();
> ...
>       Set<String> allGatewaySenderIds = getAllGatewaySenderIds();
>       if (!allGatewaySenderIds.isEmpty()) {
>         for (GatewaySender sender : this.cache.getAllGatewaySenders()) {
>           if (sender.isParallel() && allGatewaySenderIds.contains(sender.getId())) {
>             // Fix for Bug#51491. Once decided to support this configuration we have call
>             // addShadowPartitionedRegionForUserRR
>             if (sender.getId().contains(AsyncEventQueueImpl.ASYNC_EVENT_QUEUE_PREFIX)) {
>               throw new AsyncEventQueueConfigurationException(
>                   LocalizedStrings.ParallelAsyncEventQueue_0_CAN_NOT_BE_USED_WITH_REPLICATED_REGION_1
>                       .toLocalizedString(new Object[] {
>                           AsyncEventQueueImpl.getAsyncEventQueueIdFromSenderId(sender.getId()),
>                           this.getFullPath()}));
>             }
>             throw new GatewaySenderConfigurationException(
>                 LocalizedStrings.ParallelGatewaySender_0_CAN_NOT_BE_USED_WITH_REPLICATED_REGION_1
>                     .toLocalizedString(new Object[] {sender.getId(), this.getFullPath()}));
>           }
>         }
>       }
> ...
> {code}
> However, the {{gateway-sender}}/{{async-event-queue}} has been associated to the region before this last check and the code doesn't catch the {{GatewaySenderConfigurationException}} / {{AsyncEventQueueConfigurationException}} within that block:
> {code:title=GemFireCacheImpl.java|borderStyle=solid}
> public <K, V> Region<K, V> createVMRegion(String name, RegionAttributes<K, V> p_attrs, InternalRegionArguments internalRegionArgs) throws RegionExistsException, TimeoutException, IOException, ClassNotFoundException {
> ...
>       region.postCreateRegion();
>     } catch (RegionExistsException ex) {
>       // outside of sync make sure region is initialized to fix bug 37563
>       InternalRegion internalRegion = (InternalRegion) ex.getRegion();
>       internalRegion.waitOnInitialization(); // don't give out ref until initialized
>       throw ex;
>     }
> ...
> {code}
> This ends up leaving the region unusable but already added to the {{GemFireCacheImpl.rootRegions}} field, so the cache itself is left inconsistent (could be even worse when the region is also configured as persistent).
> No big deal when the user is configuring everything using a {{cache.xml}} file as the exception will be propagated and the server startup will fail. When using {{gfsh}}, on the other hand, the command {{create region}} reports a failure but leaves the region partially created on every single member.
> According to [~dschneider]
> {quote}
> The intent of postCreateRegion is to do things after the region was created. This is the wrong place to be doing validation.
> If you look at the super.postCreateRegion that DistributionRegion calls first you will see it incs stats and delivers events to others telling them the region was created. You really can't undo that well if postCreateRegion fails. So I do not think it is a good idea to have postCreateRegion destroy the region it created.
> I think the bug here is that gateway validation is in postCreateRegion. This validation should be done earlier so that the region creation it self fails and postCreateRegion is never called.
> {quote}
> The attached script reproduces the issue both using {{gfsh}} and a {{cache.xml}} file.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)