Skip to content

Conversation

@supratimdeka
Copy link
Contributor

https://issues.apache.org/jira/browse/HDDS-2206

Introduced a boolean config parameter to control exception propagation from OM to Clients.
If set to true, all system exceptions (IOException) are thrown as ServiceException to RPC client - this also propagates the complete server-side stack trace to the client. If false, system exception stack trace is logged locally on the server, not sent to client.
The default value is set to true for now. For Ozone GA, we can revisit this choice.

Business Exception (OMException) handling is not changed.

This change does not include propagation of exceptions from within Ratis server.

@elek
Copy link
Member

elek commented Oct 14, 2019

/retest

Copy link
Member

@elek elek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @supratimdeka the patch, overall it looks good to me. I like the separation between the business exception and unexpected system exception.

But can you please explain why did you introduce a new configuration settings? Do you see any risk?

I think you can just use the new code path. For me it seems to be risky to have two kind of behavior as based on the configuration the client side will receive totally different exceptions which means the code path will be totally different.

It would require full testing all the time with or without the configuration.

@supratimdeka
Copy link
Contributor Author

2 modifications in latest commit:

  1. Removed the configuration parameter. Marton had a review comment about additional complexity because of a config parameter.
    Also, making the configuration available in the validateAndUpdateCache(HDDS-2208) phase is quite complex.

  2. Exception stack trace is propagated in the message field inside OMResponse. Throwing a ServiceException directly from the validateAndUpdateCache phase is not possible because of Ratis. It is very simple to dump the exception stack trace inside the message field.
    So we are always using the message field of OMResponse for exception stacktraces, to ensure uniformity between preExecute and validateAndUpdateCache.

Copy link
Contributor

@bharatviswa504 bharatviswa504 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 LGTM.

@bharatviswa504 bharatviswa504 merged commit 445fe62 into apache:master Oct 21, 2019
asfgit pushed a commit that referenced this pull request Oct 25, 2019
…in the Ozone Manager. Contributed by Supratim Deka (#12)"

This reverts commit 445fe62.
kuenishi referenced this pull request in pfnet/ozone Feb 22, 2022
* Application errors must not flood system log

* Write log when internal error

* signing key check is for debug
tanvipenumudy added a commit to tanvipenumudy/ozone that referenced this pull request May 12, 2022
# This is the 1st commit message:

Initial Commit

# This is the commit message apache#2:

more slight changes

# This is the commit message apache#3:

changes++

# This is the commit message apache#4:

getExecutorService Changes

# This is the commit message apache#5:

applyTransaction() Changes

# This is the commit message apache#6:

changes++

# This is the commit message apache#7:

TestOzoneManagerLock changes

# This is the commit message apache#8:

add changes

# This is the commit message apache#9:

add more minor changes

# This is the commit message apache#10:

add config to ozone-default.xml

# This is the commit message apache#11:

minor changes

# This is the commit message apache#12:

change modulo logic

# This is the commit message apache#13:

changes

# This is the commit message apache#14:

changes++

# This is the commit message apache#15:

add changes++

# This is the commit message apache#16:

minor changes

# This is the commit message apache#17:

Changes (to be reverted)

# This is the commit message apache#18:

Changes 09/05
k5342 pushed a commit to k5342/ozone that referenced this pull request Oct 20, 2023
ivanzlenko pushed a commit to ivanzlenko/ozone that referenced this pull request Apr 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants