You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Bruce Schuchardt <bs...@pivotal.io> on 2016/02/20 01:43:36 UTC
Review Request 43797: GEODE-478: Message class length field overflows
if size is > 2GB
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43797/
-----------------------------------------------------------
Review request for geode, Barry Oglesby, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
Repository: geode
Description
-------
Message size is now restricted to 1GB. If this is exceeded a MessageTooLargeException is thrown.
I think the original intent of this ticket was to have messaging handle the carving up of the message into multiple Messages somehow, but I think this is the correct approach. This will ensure that the problem doesn't cause the current connection to be terminated and the operation retried on another server and make sure that the exception gets back to the point of initiation.
Barry already has a way to avoid large messages in WAN senders that can be ported to Geode.
Diffs
-----
gemfire-core/src/main/java/com/gemstone/gemfire/cache/client/internal/OpExecutorImpl.java ca14b76
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/BaseCommand.java dd13f19
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientProxy.java 69ae6d8
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Message.java 4bfd44b
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Part.java f5f6326
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/TXSynchronizationCommand.java 82e8114
gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/tier/sockets/MessageJUnitTest.java 3dc5a7d
Diff: https://reviews.apache.org/r/43797/diff/
Testing
-------
Thanks,
Bruce Schuchardt
Re: Review Request 43797: GEODE-478: Message class length field
overflows if size is > 2GB
Posted by Barry Oglesby <bo...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43797/#review120164
-----------------------------------------------------------
Ship it!
Ship It!
- Barry Oglesby
On Feb. 20, 2016, 12:43 a.m., Bruce Schuchardt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43797/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2016, 12:43 a.m.)
>
>
> Review request for geode, Barry Oglesby, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Message size is now restricted to 1GB. If this is exceeded a MessageTooLargeException is thrown.
>
> I think the original intent of this ticket was to have messaging handle the carving up of the message into multiple Messages somehow, but I think this is the correct approach. This will ensure that the problem doesn't cause the current connection to be terminated and the operation retried on another server and make sure that the exception gets back to the point of initiation.
>
> Barry already has a way to avoid large messages in WAN senders that can be ported to Geode.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/cache/client/internal/OpExecutorImpl.java ca14b76
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/BaseCommand.java dd13f19
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientProxy.java 69ae6d8
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Message.java 4bfd44b
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Part.java f5f6326
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/TXSynchronizationCommand.java 82e8114
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/tier/sockets/MessageJUnitTest.java 3dc5a7d
>
> Diff: https://reviews.apache.org/r/43797/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bruce Schuchardt
>
>
Re: Review Request 43797: GEODE-478: Message class length field
overflows if size is > 2GB
Posted by Hitesh Khamesra <hk...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43797/#review120165
-----------------------------------------------------------
Ship it!
Ship It!
- Hitesh Khamesra
On Feb. 20, 2016, 12:43 a.m., Bruce Schuchardt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43797/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2016, 12:43 a.m.)
>
>
> Review request for geode, Barry Oglesby, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Message size is now restricted to 1GB. If this is exceeded a MessageTooLargeException is thrown.
>
> I think the original intent of this ticket was to have messaging handle the carving up of the message into multiple Messages somehow, but I think this is the correct approach. This will ensure that the problem doesn't cause the current connection to be terminated and the operation retried on another server and make sure that the exception gets back to the point of initiation.
>
> Barry already has a way to avoid large messages in WAN senders that can be ported to Geode.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/cache/client/internal/OpExecutorImpl.java ca14b76
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/BaseCommand.java dd13f19
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientProxy.java 69ae6d8
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Message.java 4bfd44b
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Part.java f5f6326
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/TXSynchronizationCommand.java 82e8114
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/tier/sockets/MessageJUnitTest.java 3dc5a7d
>
> Diff: https://reviews.apache.org/r/43797/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bruce Schuchardt
>
>
Re: Review Request 43797: GEODE-478: Message class length field
overflows if size is > 2GB
Posted by Bruce Schuchardt <bs...@pivotal.io>.
> On Feb. 20, 2016, 1:02 a.m., Barry Oglesby wrote:
> > Looks good.
> >
> > One question: Why do you throw out of the writeException method?
> >
> > BaseCommand
> >
> > protected static void writeException(Message origMsg, int msgType, Throwable e,
> > ...
> > if (e instanceof MessageTooLargeException) {
> > throw (IOException)e;
> > }
I want the exception to terminate cmdExecute() methods in the command classes and get back to BaseCommand.execute(). That method handles MessageToLargeException, which is an IOException, without causing the socket to be closed.
- Bruce
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43797/#review120007
-----------------------------------------------------------
On Feb. 20, 2016, 12:43 a.m., Bruce Schuchardt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43797/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2016, 12:43 a.m.)
>
>
> Review request for geode, Barry Oglesby, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Message size is now restricted to 1GB. If this is exceeded a MessageTooLargeException is thrown.
>
> I think the original intent of this ticket was to have messaging handle the carving up of the message into multiple Messages somehow, but I think this is the correct approach. This will ensure that the problem doesn't cause the current connection to be terminated and the operation retried on another server and make sure that the exception gets back to the point of initiation.
>
> Barry already has a way to avoid large messages in WAN senders that can be ported to Geode.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/cache/client/internal/OpExecutorImpl.java ca14b76
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/BaseCommand.java dd13f19
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientProxy.java 69ae6d8
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Message.java 4bfd44b
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Part.java f5f6326
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/TXSynchronizationCommand.java 82e8114
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/tier/sockets/MessageJUnitTest.java 3dc5a7d
>
> Diff: https://reviews.apache.org/r/43797/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bruce Schuchardt
>
>
Re: Review Request 43797: GEODE-478: Message class length field
overflows if size is > 2GB
Posted by Barry Oglesby <bo...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43797/#review120007
-----------------------------------------------------------
Looks good.
One question: Why do you throw out of the writeException method?
BaseCommand
protected static void writeException(Message origMsg, int msgType, Throwable e,
...
if (e instanceof MessageTooLargeException) {
throw (IOException)e;
}
- Barry Oglesby
On Feb. 20, 2016, 12:43 a.m., Bruce Schuchardt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43797/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2016, 12:43 a.m.)
>
>
> Review request for geode, Barry Oglesby, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Message size is now restricted to 1GB. If this is exceeded a MessageTooLargeException is thrown.
>
> I think the original intent of this ticket was to have messaging handle the carving up of the message into multiple Messages somehow, but I think this is the correct approach. This will ensure that the problem doesn't cause the current connection to be terminated and the operation retried on another server and make sure that the exception gets back to the point of initiation.
>
> Barry already has a way to avoid large messages in WAN senders that can be ported to Geode.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/cache/client/internal/OpExecutorImpl.java ca14b76
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/BaseCommand.java dd13f19
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientProxy.java 69ae6d8
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Message.java 4bfd44b
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Part.java f5f6326
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/TXSynchronizationCommand.java 82e8114
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/tier/sockets/MessageJUnitTest.java 3dc5a7d
>
> Diff: https://reviews.apache.org/r/43797/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bruce Schuchardt
>
>
Re: Review Request 43797: GEODE-478: Message class length field
overflows if size is > 2GB
Posted by Barry Oglesby <bo...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43797/#review120008
-----------------------------------------------------------
After you check in, you can assign the bug to me so I can merge my changes from 820X_maint.
- Barry Oglesby
On Feb. 20, 2016, 12:43 a.m., Bruce Schuchardt wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43797/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2016, 12:43 a.m.)
>
>
> Review request for geode, Barry Oglesby, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Message size is now restricted to 1GB. If this is exceeded a MessageTooLargeException is thrown.
>
> I think the original intent of this ticket was to have messaging handle the carving up of the message into multiple Messages somehow, but I think this is the correct approach. This will ensure that the problem doesn't cause the current connection to be terminated and the operation retried on another server and make sure that the exception gets back to the point of initiation.
>
> Barry already has a way to avoid large messages in WAN senders that can be ported to Geode.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/cache/client/internal/OpExecutorImpl.java ca14b76
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/BaseCommand.java dd13f19
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/CacheClientProxy.java 69ae6d8
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Message.java 4bfd44b
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/Part.java f5f6326
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/TXSynchronizationCommand.java 82e8114
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/tier/sockets/MessageJUnitTest.java 3dc5a7d
>
> Diff: https://reviews.apache.org/r/43797/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bruce Schuchardt
>
>