[jira] [Commented] (OAK-5055) o.a.j.o.s.s.RevisionGC returns status that don't comply with the API
[ https://issues.apache.org/jira/browse/OAK-5055?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15633526#comment-15633526 ] Timothee Maret commented on OAK-5055: - Thanks [~mduerig]! I have looked at the patch and tried in my setup, LGTM. > o.a.j.o.s.s.RevisionGC returns status that don't comply with the API > > > Key: OAK-5055 > URL: https://issues.apache.org/jira/browse/OAK-5055 > Project: Jackrabbit Oak > Issue Type: Bug > Components: core >Affects Versions: 1.5.12 >Reporter: Timothee Maret >Assignee: Timothee Maret > Fix For: 1.5.13 > > Attachments: OAK-5055.patch, OAK-5055.patch, OAK-5055.patch > > > According to the API [0], the method {{RevisionGC#startRevisionGC}} should > {code} > @return the status of the operation right after it was initiated > {code} > However, it does return {{succeeded}} on every call, see [1]. > I believe this should either compute the actual state, or return > {{initiated}} or {{running}}. > Same applies for {{cancelRevisionGC}}. > [0] > https://github.com/apache/jackrabbit-oak/blob/18d69e6f590064585414f12bda724f48f3d5843f/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGCMBean.java#L34-L40 > [1] > https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java#L105 > https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java#L123 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (OAK-5055) o.a.j.o.s.s.RevisionGC returns status that don't comply with the API
[ https://issues.apache.org/jira/browse/OAK-5055?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=1560#comment-1560 ] Timothee Maret commented on OAK-5055: - No objection, thanks for looking at this! > o.a.j.o.s.s.RevisionGC returns status that don't comply with the API > > > Key: OAK-5055 > URL: https://issues.apache.org/jira/browse/OAK-5055 > Project: Jackrabbit Oak > Issue Type: Bug > Components: core >Affects Versions: 1.5.12 >Reporter: Timothee Maret >Assignee: Timothee Maret > Fix For: 1.5.13 > > Attachments: OAK-5055.patch, OAK-5055.patch, OAK-5055.patch > > > According to the API [0], the method {{RevisionGC#startRevisionGC}} should > {code} > @return the status of the operation right after it was initiated > {code} > However, it does return {{succeeded}} on every call, see [1]. > I believe this should either compute the actual state, or return > {{initiated}} or {{running}}. > Same applies for {{cancelRevisionGC}}. > [0] > https://github.com/apache/jackrabbit-oak/blob/18d69e6f590064585414f12bda724f48f3d5843f/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGCMBean.java#L34-L40 > [1] > https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java#L105 > https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java#L123 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (OAK-5055) o.a.j.o.s.s.RevisionGC returns status that don't comply with the API
[ https://issues.apache.org/jira/browse/OAK-5055?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15633290#comment-15633290 ] Michael Dürig commented on OAK-5055: I like this approach. As a small improvement I think we should bass the management operation to e.g. {{initiated}} instead of just the id. > o.a.j.o.s.s.RevisionGC returns status that don't comply with the API > > > Key: OAK-5055 > URL: https://issues.apache.org/jira/browse/OAK-5055 > Project: Jackrabbit Oak > Issue Type: Bug > Components: core >Affects Versions: 1.5.12 >Reporter: Timothee Maret >Assignee: Timothee Maret > Fix For: 1.5.13 > > Attachments: OAK-5055.patch, OAK-5055.patch, OAK-5055.patch > > > According to the API [0], the method {{RevisionGC#startRevisionGC}} should > {code} > @return the status of the operation right after it was initiated > {code} > However, it does return {{succeeded}} on every call, see [1]. > I believe this should either compute the actual state, or return > {{initiated}} or {{running}}. > Same applies for {{cancelRevisionGC}}. > [0] > https://github.com/apache/jackrabbit-oak/blob/18d69e6f590064585414f12bda724f48f3d5843f/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGCMBean.java#L34-L40 > [1] > https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java#L105 > https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java#L123 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (OAK-5055) o.a.j.o.s.s.RevisionGC returns status that don't comply with the API
[ https://issues.apache.org/jira/browse/OAK-5055?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15633221#comment-15633221 ] Michael Dürig commented on OAK-5055: Thanks for the patch. Applied at http://svn.apache.org/viewvc?view=revision=1767908 Will have a look at your other comment in a minute. > o.a.j.o.s.s.RevisionGC returns status that don't comply with the API > > > Key: OAK-5055 > URL: https://issues.apache.org/jira/browse/OAK-5055 > Project: Jackrabbit Oak > Issue Type: Bug > Components: core >Affects Versions: 1.5.12 >Reporter: Timothee Maret >Assignee: Timothee Maret > Fix For: 1.5.13 > > Attachments: OAK-5055.patch, OAK-5055.patch > > > According to the API [0], the method {{RevisionGC#startRevisionGC}} should > {code} > @return the status of the operation right after it was initiated > {code} > However, it does return {{succeeded}} on every call, see [1]. > I believe this should either compute the actual state, or return > {{initiated}} or {{running}}. > Same applies for {{cancelRevisionGC}}. > [0] > https://github.com/apache/jackrabbit-oak/blob/18d69e6f590064585414f12bda724f48f3d5843f/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGCMBean.java#L34-L40 > [1] > https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java#L105 > https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java#L123 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (OAK-5055) o.a.j.o.s.s.RevisionGC returns status that don't comply with the API
[ https://issues.apache.org/jira/browse/OAK-5055?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15633211#comment-15633211 ] Timothee Maret commented on OAK-5055: - [~mduerig] Looking deeper into the {{RevisionGC}}, it seems there's another bug. I'll discuss it here since it is coupled to the current issue. Actually returning {{initiated}}, {{failed}}, etc. is bogus, the operation id does not match the id of the {{gcOp}} field in the {{RevisionGC}} instance. This is because the operation methods implicitely increment the idGen counter (example at [0]). With the current ManagementOperation code, there seems to be no way to invoke the {{initiated}}, {{failed}}, etc. methods without messing up the id. This could be fixed however, either by 1. Exposing the {{initiated(id,message}} like methods by making them public ; and changing the {{RevisionGC}} to code to invoke those methods 2. In {{RevisionGC}}, return {{getRevisionGCStatus()}} in any case instead of {{initiated}}, {{failed}}, etc. wdyt ? [0] https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/management/ManagementOperation.java#L279 > o.a.j.o.s.s.RevisionGC returns status that don't comply with the API > > > Key: OAK-5055 > URL: https://issues.apache.org/jira/browse/OAK-5055 > Project: Jackrabbit Oak > Issue Type: Bug > Components: core >Affects Versions: 1.5.12 >Reporter: Timothee Maret >Assignee: Timothee Maret > Fix For: 1.5.13 > > Attachments: OAK-5055.patch, OAK-5055.patch > > > According to the API [0], the method {{RevisionGC#startRevisionGC}} should > {code} > @return the status of the operation right after it was initiated > {code} > However, it does return {{succeeded}} on every call, see [1]. > I believe this should either compute the actual state, or return > {{initiated}} or {{running}}. > Same applies for {{cancelRevisionGC}}. > [0] > https://github.com/apache/jackrabbit-oak/blob/18d69e6f590064585414f12bda724f48f3d5843f/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGCMBean.java#L34-L40 > [1] > https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java#L105 > https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java#L123 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (OAK-5055) o.a.j.o.s.s.RevisionGC returns status that don't comply with the API
[ https://issues.apache.org/jira/browse/OAK-5055?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15632959#comment-15632959 ] Timothee Maret commented on OAK-5055: - [~mduerig] that makes sense. Please see the updated patch > o.a.j.o.s.s.RevisionGC returns status that don't comply with the API > > > Key: OAK-5055 > URL: https://issues.apache.org/jira/browse/OAK-5055 > Project: Jackrabbit Oak > Issue Type: Bug > Components: core >Affects Versions: 1.5.12 >Reporter: Timothee Maret >Assignee: Timothee Maret > Fix For: 1.5.13 > > Attachments: OAK-5055.patch, OAK-5055.patch > > > According to the API [0], the method {{RevisionGC#startRevisionGC}} should > {code} > @return the status of the operation right after it was initiated > {code} > However, it does return {{succeeded}} on every call, see [1]. > I believe this should either compute the actual state, or return > {{initiated}} or {{running}}. > Same applies for {{cancelRevisionGC}}. > [0] > https://github.com/apache/jackrabbit-oak/blob/18d69e6f590064585414f12bda724f48f3d5843f/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGCMBean.java#L34-L40 > [1] > https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java#L105 > https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java#L123 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (OAK-5055) o.a.j.o.s.s.RevisionGC returns status that don't comply with the API
[ https://issues.apache.org/jira/browse/OAK-5055?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15632809#comment-15632809 ] Michael Dürig commented on OAK-5055: Good catch! I think we should change this to {{initiated}} to avoid any confusion when a subsequent call to {{RevisionGC.getRevisionGCStatus()}} does not (yet) return {{RUNNING}}. > o.a.j.o.s.s.RevisionGC returns status that don't comply with the API > > > Key: OAK-5055 > URL: https://issues.apache.org/jira/browse/OAK-5055 > Project: Jackrabbit Oak > Issue Type: Bug > Components: core >Affects Versions: 1.5.12 >Reporter: Timothee Maret >Assignee: Timothee Maret > Fix For: 1.5.13 > > Attachments: OAK-5055.patch > > > According to the API [0], the method {{RevisionGC#startRevisionGC}} should > {code} > @return the status of the operation right after it was initiated > {code} > However, it does return {{succeeded}} on every call, see [1]. > I believe this should either compute the actual state, or return > {{initiated}} or {{running}}. > Same applies for {{cancelRevisionGC}}. > [0] > https://github.com/apache/jackrabbit-oak/blob/18d69e6f590064585414f12bda724f48f3d5843f/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGCMBean.java#L34-L40 > [1] > https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java#L105 > https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java#L123 -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (OAK-5055) o.a.j.o.s.s.RevisionGC returns status that don't comply with the API
[ https://issues.apache.org/jira/browse/OAK-5055?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15632737#comment-15632737 ] Timothee Maret commented on OAK-5055: - [~mduerig] wdyt, should we change the implementation ? If so, shall we opt for {{initiated}} or {{running}} ? > o.a.j.o.s.s.RevisionGC returns status that don't comply with the API > > > Key: OAK-5055 > URL: https://issues.apache.org/jira/browse/OAK-5055 > Project: Jackrabbit Oak > Issue Type: Bug > Components: core >Affects Versions: 1.5.12 >Reporter: Timothee Maret >Assignee: Timothee Maret > > According to the API [0], the method {{RevisionGC#startRevisionGC}} should > {code} > @return the status of the operation right after it was initiated > {code} > However, it does return {{succeeded}} on every call, see [1]. > I believe this should either compute the actual state, or return > {{initiated}} or {{running}}. > Same applies for {{cancelRevisionGC}}. > [0] > https://github.com/apache/jackrabbit-oak/blob/18d69e6f590064585414f12bda724f48f3d5843f/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGCMBean.java#L34-L40 > [1] > https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java#L105 > https://github.com/apache/jackrabbit-oak/blob/e873825b92faf14b7ef52130c36e6466edcef6b0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java#L123 -- This message was sent by Atlassian JIRA (v6.3.4#6332)