You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by shixiaogang <gi...@git.apache.org> on 2017/02/21 16:25:09 UTC
[GitHub] flink pull request #3380: [FLINK-5865][state] Throw original exception in th...
GitHub user shixiaogang opened a pull request:
https://github.com/apache/flink/pull/3380
[FLINK-5865][state] Throw original exception in the states
The wrapping of `RuntimeException` is removed so that we can avoid redundant stack printed in the log.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/alibaba/flink flink-5865
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/3380.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 #3380
----
commit 0928c44063d5c893b24c24f21050ee643159fb36
Author: xiaogang.sxg <xi...@alibaba-inc.com>
Date: 2017-02-21T16:20:25Z
Throw original exception in RocksDB states
----
---
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] flink issue #3380: [FLINK-5865][state] Throw original exception in the state...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3380
I think you are right about the argument that we do not expect users to "handle" the exceptions in any way, so they may as well be unchecked exceptions.
Should we then throw `FlinkRuntimeException` or define a specific `StateException extends FlinkRuntimeException` that we throw there?
---
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] flink issue #3380: [FLINK-5865][state] Throw original exception in the state...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3380
Very good idea, +1
---
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] flink issue #3380: [FLINK-5865][state] Throw original exception in the state...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3380
I think it is a good idea to avoid exception wrapping where ever possible, so +1 to that.
I was wondering if we can improve exception handling even further for the state abstraction, especially getting rid of the `throws Exception` in all the methods on the state interfaces (like `ValueState.get()`).
Can we find some more "specific" exceptions for the signature, like `throws IOException` or `throws StateAccessException`. That would mean that some exceptions would have to be wrapped, though.
I think Java would support exception "downgrading", meaning going to more specific exceptions, without breaking the API.
What do you think?
---
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] flink issue #3380: [FLINK-5865][state] Throw original exception in the state...
Posted by shixiaogang <gi...@git.apache.org>.
Github user shixiaogang commented on the issue:
https://github.com/apache/flink/pull/3380
I like the idea of find some more "specific" exceptions. Flink can define some specific Exceptions like `StateAccessException`. That may help better understand the code.
I am also thinking that it's better not to put any exception in the signature of user-facing interfaces like `State`. All exceptions thrown by these methods are `RuntimeException` which are caught and handled by Flink. It's because these methods provided by Flink are supposed to work properly and do not throw any exception. Actually, users can do few things even if they can catch the exceptions.
---
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] flink issue #3380: [FLINK-5865][state] Throw original exception in the state...
Posted by shixiaogang <gi...@git.apache.org>.
Github user shixiaogang commented on the issue:
https://github.com/apache/flink/pull/3380
I prefer to throw more detailed exceptions e.g. `IncompatibleTypeSerializerException`, `StateAccessException` and `StateNotFoundException`. They all are extended from `FlinkRuntimeException`. Users can get more information from these exceptions if they catch the exceptions.
---
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] flink issue #3380: [FLINK-5865][state] Throw original exception in the state...
Posted by shixiaogang <gi...@git.apache.org>.
Github user shixiaogang commented on the issue:
https://github.com/apache/flink/pull/3380
I think we may borrow some ideas from Java. For example, the methods in `Map` do not throw any exception in their signatures. But the interfaces define a set of specific `RuntimeException` that can be thrown by the implementation such as `ClassCastException`. Maybe we can do similarly.
What do you think? @StephanEwen
---
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.
---