[jira] [Comment Edited] (IGNITE-10507) Control.sh add ability to check crc sums of stored pages

["Ivan Rakov (JIRA)" <ji...@apache.org>]

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

Ivan Rakov edited comment on IGNITE-10507 at 1/17/19 2:32 PM:
--------------------------------------------------------------

[~antonovsergey93], I had a look. A few comments:
1) 
{code:java}
                        catch (RejectedExecutionException e) {
                            assert false : "A task should never be rejected by async runner";
                        }
{code}
Such exception handling is suspicious. At least, exception will be simply ignored if assertions are disabled, which is not okay.
2) On the first glance, there's risk of ClassCastException in ValidateIndexesClosure#integrityCheckIndexPartition: cast to FilePageStoreManager will fail if group is not persistent.
3) VerifyBackupPartitionsJobV2#checkPartitionCrc and ValidateIndexesClosure#integrityCheckIndexPartition look really alike. It's up yo you, but I think we can extract common logic here.
4) 
{code:java}
                    for (int pageNo = 0; pageNo < pageStore.pages(); pageId++, pageNo++) {
                        buf.clear();

                        if (cpFlag.get())
                            throw new GridNotIdleException("Checkpoint with dirty pages started! Cluster not idle!");

                        pageStore.read(pageId, buf, true);
                    }
{code}
Reading last page is not protected by checking checkpoint flag.
5) It's a minor, but there are several places when your code lacks some spaces. Examples:
{code:java}
    @Override public String getMessage() {
        return exceptions.stream().map(e->e.getMessage()).collect(Collectors.joining(", "));
    }
{code}
{code:java}
    @Override public String toString() {
        return getClass() +": " + getMessage();
    }
{code}
{code:java}
        if(!F.isEmpty(exceptions)){
            File f = new File(IDLE_VERIFY_FILE_PREFIX + LocalDateTime.now().format(TIME_FORMATTER) + ".txt");

            try(PrintWriter pw = new PrintWriter(f)) {
{code}
{code:java}
                if(arg.isCheckCrc())
                    checkPartitionCrc(grpCtx, part, cpFlag);
{code}
and so on.


was (Author: ivan.glukos):
[~antonovsergey93], I had a look. A few comments:
1) 
{code:java}
                        catch (RejectedExecutionException e) {
                            assert false : "A task should never be rejected by async runner";
                        }
{code}
Such exception handling is suspicious. At least, exception will be simply ignored if assertions are disabled, which is not okay.
2) On the first glance, there's risk of ClassCastException in ValidateIndexesClosure#integrityCheckIndexPartition: cast to FilePageStoreManager will fail if group is not persistent.
3) VerifyBackupPartitionsJobV2#checkPartitionCrc and ValidateIndexesClosure#integrityCheckIndexPartition look really alike. It's up yo you, but I think we can extract common logic here.
4) 
{code:java}
                    for (int pageNo = 0; pageNo < pageStore.pages(); pageId++, pageNo++) {
                        buf.clear();

                        if (cpFlag.get())
                            throw new GridNotIdleException("Checkpoint with dirty pages started! Cluster not idle!");

                        pageStore.read(pageId, buf, true);
                    }
{code}
Reading last page is not protected by checking checkpoint flag.
4) It's a minor, but there are several places when your code lacks some spaces. Examples:
{code:java}
    @Override public String getMessage() {
        return exceptions.stream().map(e->e.getMessage()).collect(Collectors.joining(", "));
    }
{code}
{code:java}
    @Override public String toString() {
        return getClass() +": " + getMessage();
    }
{code}
{code:java}
        if(!F.isEmpty(exceptions)){
            File f = new File(IDLE_VERIFY_FILE_PREFIX + LocalDateTime.now().format(TIME_FORMATTER) + ".txt");

            try(PrintWriter pw = new PrintWriter(f)) {
{code}
{code:java}
                if(arg.isCheckCrc())
                    checkPartitionCrc(grpCtx, part, cpFlag);
{code}
and so on.

> Control.sh add ability to check crc sums of stored pages
> --------------------------------------------------------
>
>                 Key: IGNITE-10507
>                 URL: https://issues.apache.org/jira/browse/IGNITE-10507
>             Project: Ignite
>          Issue Type: Improvement
>    Affects Versions: 2.6
>            Reporter: Sergey Antonov
>            Assignee: Sergey Antonov
>            Priority: Critical
>             Fix For: 2.8
>
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> We should have ability to check stored data on disk. Also we should return all exceptions from all nodes, if they are occurred.



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