Skip to content

Enhance hierarchical addition on exception context#9695

Closed
bikramSingh91 wants to merge 1 commit intofacebookincubator:mainfrom
bikramSingh91:export-D56913321
Closed

Enhance hierarchical addition on exception context#9695
bikramSingh91 wants to merge 1 commit intofacebookincubator:mainfrom
bikramSingh91:export-D56913321

Conversation

@bikramSingh91
Copy link
Contributor

@bikramSingh91 bikramSingh91 commented May 2, 2024

Summary:
Currently, VeloxExceptions have the ability to add additional context
from the top most level Exception context. This is useful for
debugging exceptions thrown during expression evaluation where we
want to know the current expression that threw it and the top level
one that contains it.
With this change, the "Top-level Context" concept is replaced by
"Additional Context", and a new field called "isEssential" is added
to the ExceptionContext to specify whether the context should always
be part of the exception when traversing the hierarchy. This
modification enables adding context at various levels within the
hierarchy.
Moreover, using this new method, extra details about the operator
and relevant metadata are now included in the exception to
facilitate debugging when queries fail due to runtime exceptions.

As an example, the exception thrown while executing an operator
will look like this:

Error Source: RUNTIME
Error Code: INVALID_STATE
Reason: Throwing exception
Retriable: False
Expression: false
Context: throwexception(c0)
Additional Context: Top-level Expression: plus(c0, throwexception(c0)) Operator: FilterProject(1) PlanNodeId: 1 TaskId: test_cursor 1 PipelineId: 0 DriverId: 0 OperatorAddress: 0x61a000003c80 
Function: call
File: fbcode/velox/exec/tests/DriverTest.cpp
Line: 1529
Stack trace:

Differential Revision: D56913321

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 2, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56913321

@netlify
Copy link

netlify bot commented May 2, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit f2f8986
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/6639089c89d2fb0008e6433f

Copy link
Contributor

@mbasmanova mbasmanova left a comment

Choose a reason for hiding this comment

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

@bikramSingh91 Thank you. This should make it a bit easier to debug query failures.

bikramSingh91 pushed a commit to bikramSingh91/velox that referenced this pull request May 3, 2024
…#9695)

Summary:

Currently, VeloxExceptions have the ability to add additional context
from the top most level Exception context. This is useful for
debugging exceptions thrown during expression evaluation where we
want to know the current expression that threw it and the top level
one that contains it.
With this change, the "Top-level Context" concept is replaced by
"Additional Context", and a new field called "isEssential" is added
to the ExceptionContext to specify whether the context should always
be part of the exception when traversing the hierarchy. This
modification enables adding context at various levels within the
hierarchy.
Moreover, using this new method, extra details about the operator
and relevant metadata are now included in the exception to
facilitate debugging when queries fail due to runtime exceptions.

Reviewed By: mbasmanova

Differential Revision: D56913321
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56913321

bikramSingh91 pushed a commit to bikramSingh91/velox that referenced this pull request May 3, 2024
…#9695)

Summary:

Currently, VeloxExceptions have the ability to add additional context
from the top most level Exception context. This is useful for
debugging exceptions thrown during expression evaluation where we
want to know the current expression that threw it and the top level
one that contains it.
With this change, the "Top-level Context" concept is replaced by
"Additional Context", and a new field called "isEssential" is added
to the ExceptionContext to specify whether the context should always
be part of the exception when traversing the hierarchy. This
modification enables adding context at various levels within the
hierarchy.
Moreover, using this new method, extra details about the operator
and relevant metadata are now included in the exception to
facilitate debugging when queries fail due to runtime exceptions.

Reviewed By: mbasmanova

Differential Revision: D56913321
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56913321

bikramSingh91 pushed a commit to bikramSingh91/velox that referenced this pull request May 3, 2024
…#9695)

Summary:

Currently, VeloxExceptions have the ability to add additional context
from the top most level Exception context. This is useful for
debugging exceptions thrown during expression evaluation where we
want to know the current expression that threw it and the top level
one that contains it.
With this change, the "Top-level Context" concept is replaced by
"Additional Context", and a new field called "isEssential" is added
to the ExceptionContext to specify whether the context should always
be part of the exception when traversing the hierarchy. This
modification enables adding context at various levels within the
hierarchy.
Moreover, using this new method, extra details about the operator
and relevant metadata are now included in the exception to
facilitate debugging when queries fail due to runtime exceptions.

Reviewed By: mbasmanova

Differential Revision: D56913321
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56913321

bikramSingh91 pushed a commit to bikramSingh91/velox that referenced this pull request May 3, 2024
…#9695)

Summary:

Currently, VeloxExceptions have the ability to add additional context
from the top most level Exception context. This is useful for
debugging exceptions thrown during expression evaluation where we
want to know the current expression that threw it and the top level
one that contains it.
With this change, the "Top-level Context" concept is replaced by
"Additional Context", and a new field called "isEssential" is added
to the ExceptionContext to specify whether the context should always
be part of the exception when traversing the hierarchy. This
modification enables adding context at various levels within the
hierarchy.
Moreover, using this new method, extra details about the operator
and relevant metadata are now included in the exception to
facilitate debugging when queries fail due to runtime exceptions.

Reviewed By: mbasmanova

Differential Revision: D56913321
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56913321

bikramSingh91 pushed a commit to bikramSingh91/velox that referenced this pull request May 3, 2024
…#9695)

Summary:

Currently, VeloxExceptions have the ability to add additional context
from the top most level Exception context. This is useful for
debugging exceptions thrown during expression evaluation where we
want to know the current expression that threw it and the top level
one that contains it.
With this change, the "Top-level Context" concept is replaced by
"Additional Context", and a new field called "isEssential" is added
to the ExceptionContext to specify whether the context should always
be part of the exception when traversing the hierarchy. This
modification enables adding context at various levels within the
hierarchy.
Moreover, using this new method, extra details about the operator
and relevant metadata are now included in the exception to
facilitate debugging when queries fail due to runtime exceptions.

Reviewed By: mbasmanova

Differential Revision: D56913321
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56913321

bikramSingh91 pushed a commit to bikramSingh91/velox that referenced this pull request May 3, 2024
…#9695)

Summary:

Currently, VeloxExceptions have the ability to add additional context
from the top most level Exception context. This is useful for
debugging exceptions thrown during expression evaluation where we
want to know the current expression that threw it and the top level
one that contains it.
With this change, the "Top-level Context" concept is replaced by
"Additional Context", and a new field called "isEssential" is added
to the ExceptionContext to specify whether the context should always
be part of the exception when traversing the hierarchy. This
modification enables adding context at various levels within the
hierarchy.
Moreover, using this new method, extra details about the operator
and relevant metadata are now included in the exception to
facilitate debugging when queries fail due to runtime exceptions.

Reviewed By: mbasmanova

Differential Revision: D56913321
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56913321

…#9695)

Summary:

Currently, VeloxExceptions have the ability to add additional context
from the top most level Exception context. This is useful for
debugging exceptions thrown during expression evaluation where we
want to know the current expression that threw it and the top level
one that contains it.
With this change, the "Top-level Context" concept is replaced by
"Additional Context", and a new field called "isEssential" is added
to the ExceptionContext to specify whether the context should always
be part of the exception when traversing the hierarchy. This
modification enables adding context at various levels within the
hierarchy.
Moreover, using this new method, extra details about the operator
and relevant metadata are now included in the exception to
facilitate debugging when queries fail due to runtime exceptions.

Reviewed By: mbasmanova

Differential Revision: D56913321
bikramSingh91 pushed a commit to bikramSingh91/velox that referenced this pull request May 6, 2024
…#9695)

Summary:

Currently, VeloxExceptions have the ability to add additional context
from the top most level Exception context. This is useful for
debugging exceptions thrown during expression evaluation where we
want to know the current expression that threw it and the top level
one that contains it.
With this change, the "Top-level Context" concept is replaced by
"Additional Context", and a new field called "isEssential" is added
to the ExceptionContext to specify whether the context should always
be part of the exception when traversing the hierarchy. This
modification enables adding context at various levels within the
hierarchy.
Moreover, using this new method, extra details about the operator
and relevant metadata are now included in the exception to
facilitate debugging when queries fail due to runtime exceptions.

Reviewed By: mbasmanova

Differential Revision: D56913321
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56913321

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56913321

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 5bf1e27.

Joe-Abraham pushed a commit to Joe-Abraham/velox that referenced this pull request May 8, 2024
…#9695)

Summary:
Pull Request resolved: facebookincubator#9695

Currently, VeloxExceptions have the ability to add additional context
from the top most level Exception context. This is useful for
debugging exceptions thrown during expression evaluation where we
want to know the current expression that threw it and the top level
one that contains it.
With this change, the "Top-level Context" concept is replaced by
"Additional Context", and a new field called "isEssential" is added
to the ExceptionContext to specify whether the context should always
be part of the exception when traversing the hierarchy. This
modification enables adding context at various levels within the
hierarchy.
Moreover, using this new method, extra details about the operator
and relevant metadata are now included in the exception to
facilitate debugging when queries fail due to runtime exceptions.

Reviewed By: mbasmanova

Differential Revision: D56913321

fbshipit-source-id: abc56abb72b0e526d8858232031279ad7a72a238
Joe-Abraham pushed a commit to Joe-Abraham/velox that referenced this pull request Jun 7, 2024
…#9695)

Summary:
Pull Request resolved: facebookincubator#9695

Currently, VeloxExceptions have the ability to add additional context
from the top most level Exception context. This is useful for
debugging exceptions thrown during expression evaluation where we
want to know the current expression that threw it and the top level
one that contains it.
With this change, the "Top-level Context" concept is replaced by
"Additional Context", and a new field called "isEssential" is added
to the ExceptionContext to specify whether the context should always
be part of the exception when traversing the hierarchy. This
modification enables adding context at various levels within the
hierarchy.
Moreover, using this new method, extra details about the operator
and relevant metadata are now included in the exception to
facilitate debugging when queries fail due to runtime exceptions.

Reviewed By: mbasmanova

Differential Revision: D56913321

fbshipit-source-id: abc56abb72b0e526d8858232031279ad7a72a238
bikramSingh91 added a commit to bikramSingh91/presto that referenced this pull request Jun 17, 2024
Ensure inclusion of "additional context" from Velox exceptions in
the translation to Presto's ExecutionFailureInfo (which is used
for propagating the query failure error to the coordinator). See
Velox PR (facebookincubator/velox#9695)
for more details on "additional context"

Test Plan:
Enhanced existing unit test.
aditi-pandit pushed a commit to prestodb/presto that referenced this pull request Jun 17, 2024
Ensure inclusion of "additional context" from Velox exceptions in
the translation to Presto's ExecutionFailureInfo (which is used
for propagating the query failure error to the coordinator). See
Velox PR (facebookincubator/velox#9695)
for more details on "additional context"

Test Plan:
Enhanced existing unit test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants