[jira] [Commented] (IGNITE-7831) Throw Exceptions instead of AssertionErrors when reading from corrupted persistence

["Aleksey Plekhanov (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/IGNITE-7831?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16383714#comment-16383714 ] 

Aleksey Plekhanov commented on IGNITE-7831:
-------------------------------------------

[~ilantukh], I search for assertions in the persistance implementation, there are about 7 hundreds occurrencies, but almost all of this assertions are used to ensure internal logic and not depend on external reasons.

Also, most of explicitly thrown AssertionErrors in the persistance implementation are rethrown assertions with additional information. 

I found only couple of cases which can potentialy match your description:

{{PageMemoryImpl#tryToRestorePage:}}
{code:java}
throw new AssertionError(String.format(
    "Page is broken. Can't restore it from WAL. (grpId = %d, pageId = %X).",
    fullId.groupId(), fullId.pageId()
));
{code}
{{GridTreePrinter#getChildren}}: 
{code:java}
            catch (IgniteCheckedException ignored) {
                throw new AssertionError("Can not acquire page.");
            }
{code}
But this method used only for dumping debug information in the test framework.
  
 {{FilePageStoreManager#beforeCacheGroupStart}}:
{code:java}
assert dir.exists();
{code}
{{FilePageStoreManager#storeCacheData}}:
{code:java}
assert cacheWorkDir.exists() : "Work directory does not exist: " + cacheWorkDir;
{code}
But this method creates dir (and checks result code) before assertion.

Did you mean these cases or something else? If something else, do you have stacktraces or any description of real cases?

> Throw Exceptions instead of AssertionErrors when reading from corrupted persistence
> -----------------------------------------------------------------------------------
>
>                 Key: IGNITE-7831
>                 URL: https://issues.apache.org/jira/browse/IGNITE-7831
>             Project: Ignite
>          Issue Type: Improvement
>            Reporter: Ilya Lantukh
>            Assignee: Aleksey Plekhanov
>            Priority: Major
>              Labels: iep-14
>             Fix For: 2.5
>
>
> There are a few places in our code where we explicitly throw AssertionErrors due to inability to correctly read data from persistence and many more places where we make assertions based on read values.
> Assertions are used to indicate problems in internal logic, while persistence might also get corrupted by various external reasons. It also makes uniform handling of such issues considerably harder, because exception handling logic in Ignite ignores Errors. If we want to improve stability and minimize consequenses of pesistence corruption, we should replace all those AssertionErrors and asserts with Exceptions, so that current exception handling mechanisms could be reduce. In a number of situations it means that instead of causing cluster-wide hang-up problematic node will be automatically killed.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)