You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by shroman <gi...@git.apache.org> on 2015/11/05 10:00:49 UTC
[GitHub] incubator-geode pull request: [GEODE-252] Remove deprecated Partit...
GitHub user shroman opened a pull request:
https://github.com/apache/incubator-geode/pull/29
[GEODE-252] Remove deprecated PartitionAttributes methods
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shroman/incubator-geode geode-252
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-geode/pull/29.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #29
----
commit 47e699674b0baf232288f3bc69ed6032423fa41e
Author: shtykh_roman <sh...@cyberagent.co.jp>
Date: 2015-11-05T08:39:09Z
geode-252
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request: [GEODE-252] Remove deprecated Partit...
Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on the pull request:
https://github.com/apache/incubator-geode/pull/29#issuecomment-154695629
Yes, I was not sure it is safe. I updated it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request: [GEODE-252] Remove deprecated Partit...
Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/29#issuecomment-155212542
Ok, looks good. I will run some tests and merge this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request: [GEODE-252] Remove deprecated Partit...
Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/29#issuecomment-154219832
Looks good! I noticed you left in the localProperties and globalProperties fields on PartitionAttributesImpl, as well as leaving the toData/fromData methods alone in that class. Was that to avoid breaking serialization compatibility?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Re: [GitHub] incubator-geode pull request: [GEODE-252] Remove
deprecated Partit...
Posted by Dan Smith <ds...@pivotal.io>.
Just to clarify my understanding if what we are and are not trying to
support here:
1. We are breaking P2P compatibility between old versions of gemfire and
geode 1.0
2. We are trying not to break
- client server compatibility?
- disk compatibility?
- WAN compatibility?
For this particular pull request, I think the changes will only effect P2P
messaging. I don't think instances of PartitionAttributesImpl are sent to
clients. I suppose we could still add the serialization methods for
compatibility with old versions just to be safe, as described here -
https://cwiki.apache.org/confluence/display/GEODE/Managing+Backward+Compatibility
.
On Thu, Nov 12, 2015 at 12:52 AM, Michael Stolz <ms...@pivotal.io> wrote:
> We have to be very careful about how much backward compatibility we break.
> Bad enough we have to do a full cluster restart, but if we also lose all
> the data because of serialization changes that's a problem.
> When we make a breaking protocol change the mechanism for releasing the new
> code is
> 1. Ensure that there is a working back-up with WAN Gateway set up to feed
> the system being upgraded.
> 2. Bring down the system to be upgraded
> 3. Deploy the upgrade
> 4. Restart the upgraded system
> 5. Once the system has completed GII, start the Gateway receivers so that
> it can catch up on anything it missed while it was down.
>
> This mechanism has to be kept intact.
>
> This would also break the clients for much the same reason, they would all
> have to be upgraded at the same time as the servers. That has NEVER been
> possible.
>
>
> --
> Mike Stolz
> Principal Engineer, GemFire Product Manager
> Mobile: 631-835-4771
>
> On Wed, Nov 11, 2015 at 5:11 PM, upthewaterspout <gi...@git.apache.org>
> wrote:
>
> > Github user upthewaterspout commented on the pull request:
> >
> >
> > https://github.com/apache/incubator-geode/pull/29#issuecomment-155849235
> >
> > I hit some failures with these changes. I think these related to
> tests
> > that are using these features that probably need to get fixed.
> >
> >
> > com.gemstone.gemfire.cache30.CacheXml45DUnitTest.testPartitionedRegionXML
> >
> >
> com.gemstone.gemfire.codeAnalysis.AnalyzeSerializablesJUnitTest.testDataSerializables
> >
> >
> com.gemstone.gemfire.internal.cache.PartitionedRegionCacheXMLExampleDUnitTest.testExampleWithSubRegion
> >
> >
> com.gemstone.gemfire.internal.cache.PartitionedRegionCreationDUnitTest.testTotalNumberOfBuckets
> >
> > The AnalyzeSerializables is failing because it detected a change to a
> > serializable class. I think since we are breaking backwards compatibility
> > with older versions of gemfire that's fine, it probably just needs an
> > update to sanctionedSerializables.
> >
> >
> > ---
> > If your project is set up for it, you can reply to this email and have
> your
> > reply appear on GitHub as well. If your project does not have this
> feature
> > enabled and wishes so, or if the feature is enabled but not working,
> please
> > contact infrastructure at infrastructure@apache.org or file a JIRA
> ticket
> > with INFRA.
> > ---
> >
>
Re: [GitHub] incubator-geode pull request: [GEODE-252] Remove
deprecated Partit...
Posted by Michael Stolz <ms...@pivotal.io>.
We have to be very careful about how much backward compatibility we break.
Bad enough we have to do a full cluster restart, but if we also lose all
the data because of serialization changes that's a problem.
When we make a breaking protocol change the mechanism for releasing the new
code is
1. Ensure that there is a working back-up with WAN Gateway set up to feed
the system being upgraded.
2. Bring down the system to be upgraded
3. Deploy the upgrade
4. Restart the upgraded system
5. Once the system has completed GII, start the Gateway receivers so that
it can catch up on anything it missed while it was down.
This mechanism has to be kept intact.
This would also break the clients for much the same reason, they would all
have to be upgraded at the same time as the servers. That has NEVER been
possible.
--
Mike Stolz
Principal Engineer, GemFire Product Manager
Mobile: 631-835-4771
On Wed, Nov 11, 2015 at 5:11 PM, upthewaterspout <gi...@git.apache.org> wrote:
> Github user upthewaterspout commented on the pull request:
>
>
> https://github.com/apache/incubator-geode/pull/29#issuecomment-155849235
>
> I hit some failures with these changes. I think these related to tests
> that are using these features that probably need to get fixed.
>
>
> com.gemstone.gemfire.cache30.CacheXml45DUnitTest.testPartitionedRegionXML
>
> com.gemstone.gemfire.codeAnalysis.AnalyzeSerializablesJUnitTest.testDataSerializables
>
> com.gemstone.gemfire.internal.cache.PartitionedRegionCacheXMLExampleDUnitTest.testExampleWithSubRegion
>
> com.gemstone.gemfire.internal.cache.PartitionedRegionCreationDUnitTest.testTotalNumberOfBuckets
>
> The AnalyzeSerializables is failing because it detected a change to a
> serializable class. I think since we are breaking backwards compatibility
> with older versions of gemfire that's fine, it probably just needs an
> update to sanctionedSerializables.
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
>
[GitHub] incubator-geode pull request: [GEODE-252] Remove deprecated Partit...
Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on the pull request:
https://github.com/apache/incubator-geode/pull/29#issuecomment-155849235
I hit some failures with these changes. I think these related to tests that are using these features that probably need to get fixed.
com.gemstone.gemfire.cache30.CacheXml45DUnitTest.testPartitionedRegionXML
com.gemstone.gemfire.codeAnalysis.AnalyzeSerializablesJUnitTest.testDataSerializables
com.gemstone.gemfire.internal.cache.PartitionedRegionCacheXMLExampleDUnitTest.testExampleWithSubRegion
com.gemstone.gemfire.internal.cache.PartitionedRegionCreationDUnitTest.testTotalNumberOfBuckets
The AnalyzeSerializables is failing because it detected a change to a serializable class. I think since we are breaking backwards compatibility with older versions of gemfire that's fine, it probably just needs an update to sanctionedSerializables.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---