The dead code itself wasnt the issue raised, it's that the code created and operated on a buffer that remains even after removing the dead methods and is still being referenced elsewhere, though the field is never populated now so the using getData() method can only ever throw, and that method seems like it can be called.
If you are saying that the getData() method can never actually be called despite appearance otherwise, then at minimum its impl should be changed to indicate that instead and stop referencing a field that is never populated. On Tue, 14 Sept 2021 at 14:00, Clebert Suconic <clebert.suco...@gmail.com> wrote: > > I wasn't sure what you were referring on that PR.... I thought you > still needed changes on the large message processing... > > > As for the dead code.. at some point I was reading the messages from > disk during reload of the journal. I then changed and I always have > the first packet on memory and I use that one to parse properties and > annotations. > > For converters... on AMQPLargeMessage.toCore() I will use the same > method used to deliver a large message (the Deliverer Context).. so > those dead code were forgotten there and not needed... there's nothing > to be done in that case. > > I thought I already mentioned that in the PR. > > On Tue, Sep 14, 2021 at 4:13 AM Robbie Gemmell <robbie.gemm...@gmail.com> > wrote: > > > > Revert what exactly? Reverting the PR this was raised on wouldn't > > actually change anything, since the apparent issue isnt to do with the > > changes made in the PR (other than it removed the dead code that looks > > like it shouldnt be dead, since the buffer it opened/closed is still > > referenced). Its some earlier change that must have made it the way it > > is. > > > > On Mon, 13 Sept 2021 at 22:43, Clebert Suconic > > <clebert.suco...@gmail.com> wrote: > > > > > > If the large message delivering in AMQP still an issue, I would rather > > > just revert the whole thing and keep the way it was before.. I thought > > > it was a simple change. > > > I'm addressing another issue I'm working on.. and I won't be able to > > > look into that > > > > > > > > > @Franz / @Robbie Gemmell ? > > > > > > > > > > > > On Mon, Sep 13, 2021 at 4:37 AM Robbie Gemmell <robbie.gemm...@gmail.com> > > > wrote: > > > > > > > > I this also still needs looked at: > > > > > > > > > I also think the issue noted in these comments should be investigated > > > > > before another release occurs: > > > > > https://github.com/apache/activemq-artemis/pull/3711#issuecomment-913981275 > > > > > https://github.com/apache/activemq-artemis/pull/3711#issuecomment-914286613 > > > > > https://github.com/apache/activemq-artemis/pull/3711#discussion_r699397216 > > > > > > > > > On Fri, 10 Sept 2021 at 23:50, Clebert Suconic > > > > <clebert.suco...@gmail.com> wrote: > > > > > > > > > > I have merged the change on the mirror / paging fix.... > > > > > > > > > > > > > > > Before I release though I need to remove the CLI input I added.. some > > > > > people complained about the retention input I asked during the > > > > > create.. (some users were saying their scripts were broken)... > > > > > although we have the --silent for such cases. > > > > > > > > > > > > > > > > > > > > On Thu, Sep 9, 2021 at 10:51 PM Clebert Suconic > > > > > <clebert.suco...@gmail.com> wrote: > > > > > > > > > > > > I'm doing a retry on the same thread as the depage executor. > > > > > > > > > > > > > > > > > > I am finishing a test I'm writing and I will send the PR tomorrow > > > > > > (Friday) > > > > > > > > > > > > On Thu, Sep 9, 2021 at 12:46 PM Robbie Gemmell > > > > > > <robbie.gemm...@gmail.com> wrote: > > > > > > > > > > > > > > I left some mostly trivial feedback on another PR merged > > > > > > > yesterday, > > > > > > > #3728, but there is one comment I think also needs looked at > > > > > > > first: > > > > > > > https://github.com/apache/activemq-artemis/pull/3728#discussion_r705417703 > > > > > > > > > > > > > > On Thu, 9 Sept 2021 at 15:26, Clebert Suconic > > > > > > > <clebert.suco...@gmail.com> wrote: > > > > > > > > > > > > > > > > Let’s postpone until Monday while we investigate these. > > > > > > > > > > > > > > > > On Thu, Sep 9, 2021 at 9:01 AM Robbie Gemmell > > > > > > > > <robbie.gemm...@gmail.com> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Franz believes he found the issue, side effects from an > > > > > > > > > earlier change > > > > > > > > > made months ago being hit now, exposed by use of the affected > > > > > > > > > method > > > > > > > > > in recent changes. > > > > > > > > > https://issues.apache.org/jira/browse/ARTEMIS-3465 > > > > > > > > > https://github.com/apache/activemq-artemis/pull/3731 > > > > > > > > > > > > > > > > > > I also think the issue noted in these comments should be > > > > > > > > > investigated > > > > > > > > > before another release occurs: > > > > > > > > > https://github.com/apache/activemq-artemis/pull/3711#issuecomment-913981275 > > > > > > > > > https://github.com/apache/activemq-artemis/pull/3711#issuecomment-914286613 > > > > > > > > > https://github.com/apache/activemq-artemis/pull/3711#discussion_r699397216 > > > > > > > > > > > > > > > > > > On Thu, 9 Sept 2021 at 12:57, Robbie Gemmell > > > > > > > > > <robbie.gemm...@gmail.com> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > Changes made on main recently (perhaps yesterday) look to > > > > > > > > > > have rather > > > > > > > > > > broken some things on expanded test runs (i.e not the > > > > > > > > > > push/PR subset), > > > > > > > > > > so that needs to be resolved first. > > > > > > > > > > > > > > > > > > > > On Wed, 8 Sept 2021 at 22:48, Clebert Suconic > > > > > > > > > > <clebert.suco...@gmail.com> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > Any problem if I did the release tomorrow? > > > > > > > > > > > > > > > > > > > > > > Anyone wants to include anything extra ? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Aug 20, 2021 at 3:10 PM Clebert Suconic < > > > > > > > > > clebert.suco...@gmail.com> > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > I would like to do a 2.19.0 release around Aug-30th. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Please help me out on merging stuff required before > > > > > > > > > > > > then. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > if there's something (JIRA or PR) you would really > > > > > > > > > > > > like to include > > > > > > > > > > > > please mention it here.. (Last time someone mentioned > > > > > > > > > > > > me on the PR I > > > > > > > > > > > > missed the notification). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > Clebert Suconic > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > Clebert Suconic > > > > > > > > > > > > > > > > > -- > > > > > > > > Clebert Suconic > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Clebert Suconic > > > > > > > > > > > > > > > > > > > > -- > > > > > Clebert Suconic > > > > > > > > > > > > -- > > > Clebert Suconic > > > > -- > Clebert Suconic
