[jira] [Commented] (OAK-5590) The check command doesn't do any check when "deep" option is not provided
[
https://issues.apache.org/jira/browse/OAK-5590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15854272#comment-15854272
]
Andrei Dulceanu commented on OAK-5590:
--
Please disregard the comment above. After discussing about this offline with
[~mduerig] I understood better the reasoning behind {{badPaths}}.
> The check command doesn't do any check when "deep" option is not provided
> -
>
> Key: OAK-5590
> URL: https://issues.apache.org/jira/browse/OAK-5590
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: run, segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
> Labels: tooling
> Fix For: 1.8, 1.6.1
>
> Attachments: OAK-5590.patch
>
>
> When the {{check}} command is used without {{--deep}} option, there is no
> check/traversal being done against the repository.
> First relevant line in code is [1], where a check is supposed to happen, but
> due to a mismatch between argument expected/argument provided, {{null}} is
> always returned without checking anything. The method which should do the
> actual check [2] expects a set of paths to be traversed, but this set is
> always empty. Therefore, relevant code for running the check is never
> executed [3].
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L120
> [2]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L183
> [3]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L194
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (OAK-5590) The check command doesn't do any check when "deep" option is not provided
[
https://issues.apache.org/jira/browse/OAK-5590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15854203#comment-15854203
]
Andrei Dulceanu commented on OAK-5590:
--
bq. AFAIKS this would trigger a full traversal
Not necessarily, since {{checkPath}} always does a traversal with {{deep}} set
to {{false}} [1].
bq. The reason for tracking bad path internally is performance: fully
traversing every revisions from the journal takes way to long.
I understand the reasoning, but it is still unclear to me how this is applied
in the current code. The bad path logic is/will be applied only for the root
node anyway (in a scenario in which there's a problem with it in the latest
revision, its path is added in {{badPaths}} which is checked again
in subsequent revisions, until we find a consistent revision). The changes
proposed in the patch are in line with this expectation and also keep the
current expectation of doing/not doing a full traversal when {{--deep}} is
specified/not specified.
[1]
https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L200
> The check command doesn't do any check when "deep" option is not provided
> -
>
> Key: OAK-5590
> URL: https://issues.apache.org/jira/browse/OAK-5590
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: run, segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
> Labels: tooling
> Fix For: 1.8, 1.6.1
>
> Attachments: OAK-5590.patch
>
>
> When the {{check}} command is used without {{--deep}} option, there is no
> check/traversal being done against the repository.
> First relevant line in code is [1], where a check is supposed to happen, but
> due to a mismatch between argument expected/argument provided, {{null}} is
> always returned without checking anything. The method which should do the
> actual check [2] expects a set of paths to be traversed, but this set is
> always empty. Therefore, relevant code for running the check is never
> executed [3].
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L120
> [2]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L183
> [3]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L194
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (OAK-5590) The check command doesn't do any check when "deep" option is not provided
[
https://issues.apache.org/jira/browse/OAK-5590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15854170#comment-15854170
]
Michael Dürig commented on OAK-5590:
bq. I noticed that when {{\--deep}} is not specified there is no check being
performed
Ah see it now.
bq. Would it make sense to consider the root node a candidate to be included in
{{badPaths}}
AFAIKS this would trigger a full traversal. I think we should rather remove
{{\--deep}} and make full traversal the default. Only checking accessibility of
the root nodes doesn't make much sense. Even more so because the file store
automatically rolls back on startup if a root revisions is not accessible. In
terms of not doing full traversals it is more interesting to restrict by path
(aka OAK-5556).
> The check command doesn't do any check when "deep" option is not provided
> -
>
> Key: OAK-5590
> URL: https://issues.apache.org/jira/browse/OAK-5590
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: run, segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
> Labels: tooling
> Fix For: 1.8, 1.6.1
>
> Attachments: OAK-5590.patch
>
>
> When the {{check}} command is used without {{--deep}} option, there is no
> check/traversal being done against the repository.
> First relevant line in code is [1], where a check is supposed to happen, but
> due to a mismatch between argument expected/argument provided, {{null}} is
> always returned without checking anything. The method which should do the
> actual check [2] expects a set of paths to be traversed, but this set is
> always empty. Therefore, relevant code for running the check is never
> executed [3].
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L120
> [2]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L183
> [3]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L194
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (OAK-5590) The check command doesn't do any check when "deep" option is not provided
[
https://issues.apache.org/jira/browse/OAK-5590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15854151#comment-15854151
]
Andrei Dulceanu commented on OAK-5590:
--
[~mduerig] re. first bullet point, this change is tracked in OAK-5556. I only
mentioned it before, because I was working on both, and saw how the changes
proposed in the patch would influence the solution for the other issue.
Re. 2nd bullet point, I noticed that when {{--deep}} is not specified there is
no check being performed. That said, there might be a flaw in the algorithm
explained, because initially the set of {{badPaths}} is empty. Would it make
sense to consider the root node a candidate to be included in {{badPaths}}, in
order to be sure that at least one iteration of the algorithm will be executed?
> The check command doesn't do any check when "deep" option is not provided
> -
>
> Key: OAK-5590
> URL: https://issues.apache.org/jira/browse/OAK-5590
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: run, segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
> Labels: tooling
> Fix For: 1.8, 1.6.1
>
> Attachments: OAK-5590.patch
>
>
> When the {{check}} command is used without {{--deep}} option, there is no
> check/traversal being done against the repository.
> First relevant line in code is [1], where a check is supposed to happen, but
> due to a mismatch between argument expected/argument provided, {{null}} is
> always returned without checking anything. The method which should do the
> actual check [2] expects a set of paths to be traversed, but this set is
> always empty. Therefore, relevant code for running the check is never
> executed [3].
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L120
> [2]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L183
> [3]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L194
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (OAK-5590) The check command doesn't do any check when "deep" option is not provided
[
https://issues.apache.org/jira/browse/OAK-5590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15854137#comment-15854137
]
Alex Parvulescu commented on OAK-5590:
--
bq. To that respect adding a new command line flag where you could restrict the
set of paths to traverse would be a new feature. Let's discuss and track this
in a separate issue from this one.
OAK-5556
> The check command doesn't do any check when "deep" option is not provided
> -
>
> Key: OAK-5590
> URL: https://issues.apache.org/jira/browse/OAK-5590
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: run, segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
> Labels: tooling
> Fix For: 1.8, 1.6.1
>
> Attachments: OAK-5590.patch
>
>
> When the {{check}} command is used without {{--deep}} option, there is no
> check/traversal being done against the repository.
> First relevant line in code is [1], where a check is supposed to happen, but
> due to a mismatch between argument expected/argument provided, {{null}} is
> always returned without checking anything. The method which should do the
> actual check [2] expects a set of paths to be traversed, but this set is
> always empty. Therefore, relevant code for running the check is never
> executed [3].
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L120
> [2]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L183
> [3]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L194
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (OAK-5590) The check command doesn't do any check when "deep" option is not provided
[
https://issues.apache.org/jira/browse/OAK-5590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15854136#comment-15854136
]
Michael Dürig commented on OAK-5590:
[~dulceanu], [~frm] let me clarify a couple of apparent misconceptions re.
{{check}}.
* The goal of {{check}} was to check a repository for inconsistencies and find
the most recent consistent version. To that respect adding a new command line
flag where you could restrict the set of paths to traverse would be a new
feature. Let's discuss and track this in a separate issue from this one.
* The reason for tracking bad path internally is performance: fully traversing
every revisions from the journal takes *way* to long. The initial logic was:
## set {{rev}} to the most recent revision from {{journal.log}}
## traverse {{rev}} until a {{SNFE}} occurs and remember the paths where the
{{SNFE}} occurred as bad path. Otherwise {{rev}} is the most recent good
revision.
## go one revision back and traverse all bad paths. Repeat this step until no
{{SNFE}} occurs.
## repeat from step 2.
* Not specifying {{\--deep}} means that no traversal would be done but just the
root nodes would be checked. This is probably not helpful and we might want to
change it. My preference would be to always traverse regardless of {{\--deep}}.
If the option is specified we could just print a warning about the option being
discontinued and ignored and tell the user that leaving it out would already do
what {{\--deep}} did before.
* For any changes in semantics of the command line options we need to be aware
that there are a lot of command lines around (in Wikis, FAQs, emails, etc.)
where people just copy paste from. If this happens the tool should either still
do what the user expects or fail fast.
> The check command doesn't do any check when "deep" option is not provided
> -
>
> Key: OAK-5590
> URL: https://issues.apache.org/jira/browse/OAK-5590
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: run, segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
> Labels: tooling
> Fix For: 1.8, 1.6.1
>
> Attachments: OAK-5590.patch
>
>
> When the {{check}} command is used without {{--deep}} option, there is no
> check/traversal being done against the repository.
> First relevant line in code is [1], where a check is supposed to happen, but
> due to a mismatch between argument expected/argument provided, {{null}} is
> always returned without checking anything. The method which should do the
> actual check [2] expects a set of paths to be traversed, but this set is
> always empty. Therefore, relevant code for running the check is never
> executed [3].
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L120
> [2]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L183
> [3]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L194
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (OAK-5590) The check command doesn't do any check when "deep" option is not provided
[
https://issues.apache.org/jira/browse/OAK-5590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15853983#comment-15853983
]
Francesco Mari commented on OAK-5590:
-
Since there is no way to populate {{badPaths}} from the CLI, the algorithm
implemented by the consistency checker should probably be changed as follow.
{noformat}
for each revision in journal {
badPath = traverse(revision)
if badPath != null {
print revision, badPath
} else {
return revision
}
}
{noformat}
This makes the {{\--deep}} option obsolete because a deep traversal of the
content tree is always performed.
I don't see how {{badPaths}} can be useful in the current implementation. A
broken path in a revision might not be broken in an earlier one. As such, to
find a good revision that is genuinely good a full traversal is required in
every iteration of the loop.
> The check command doesn't do any check when "deep" option is not provided
> -
>
> Key: OAK-5590
> URL: https://issues.apache.org/jira/browse/OAK-5590
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: run, segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
> Labels: tooling
> Fix For: 1.8, 1.6.1
>
>
> When the {{check}} command is used without {{--deep}} option, there is no
> check/traversal being done against the repository.
> First relevant line in code is [1], where a check is supposed to happen, but
> due to a mismatch between argument expected/argument provided, {{null}} is
> always returned without checking anything. The method which should do the
> actual check [2] expects a set of paths to be traversed, but this set is
> always empty. Therefore, relevant code for running the check is never
> executed [3].
> [1]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L120
> [2]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L183
> [3]
> https://github.com/apache/jackrabbit-oak/blob/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java#L194
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
