You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/07/06 18:19:50 UTC

[GitHub] [activemq-artemis] clebertsuconic opened a new pull request #3644: ARTEMIS-3327 Removing unecessary block oepration on journal append record

clebertsuconic opened a new pull request #3644:
URL: https://github.com/apache/activemq-artemis/pull/3644


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on pull request #3644: ARTEMIS-3327 Removing unecessary block operation on journal append record

Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #3644:
URL: https://github.com/apache/activemq-artemis/pull/3644#issuecomment-876255354


   Revert and test addition were done via #3647 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] asfgit closed pull request #3644: ARTEMIS-3327 Removing unecessary block operation on journal append record

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3644:
URL: https://github.com/apache/activemq-artemis/pull/3644


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3644: ARTEMIS-3327 Removing unecessary block operation on journal append record

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3644:
URL: https://github.com/apache/activemq-artemis/pull/3644#issuecomment-875733461


   @gemmellr / @brusdev / @jbertram I was thinking only about the artemis use case here, where we always use this method with sync=false.
   
   I know some people is using the journal directly (who actually requested me to make the journal an independent project a few times).. and they actually will use the sync version.
   
   
   I will just revert this method. thanks for the comments.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] brusdev commented on pull request #3644: ARTEMIS-3327 Removing unecessary block operation on journal append record

Posted by GitBox <gi...@apache.org>.
brusdev commented on pull request #3644:
URL: https://github.com/apache/activemq-artemis/pull/3644#issuecomment-875570269


   @clebertsuconic was the result used to wait the appendExecutor execution?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3644: ARTEMIS-3327 Removing unecessary block operation on journal append record

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3644:
URL: https://github.com/apache/activemq-artemis/pull/3644#issuecomment-875041836


   @jbertram that's right


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on pull request #3644: ARTEMIS-3327 Removing unecessary block operation on journal append record

Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #3644:
URL: https://github.com/apache/activemq-artemis/pull/3644#issuecomment-875599939


   Depends on the provided values of 'sync' and 'callback'. The called _newSyncAndCallbackResult(sync, callback)_ method creating the future did this:
   `return (sync && callback == null) ? new SimpleFutureImpl<>() : SimpleFuture.dumb();`
   
   If asked to sync and there was no callback, it returns a 'proper future' that could be used to actually wait for the execution, and detect failures from thrown exceptions passed via the fail method.
   
   If not asked to sync, or a callback is given, it returns a 'dumb' future that doesnt wait at all and just returns null immediately, ignores completions/failures etc.
   
   I dont know what the sync and callback values actually are for this situation but I guess from that they were eitehr always false or non-null. Might be worth adding a check to that effect though.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] gemmellr commented on pull request #3644: ARTEMIS-3327 Removing unecessary block operation on journal append record

Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #3644:
URL: https://github.com/apache/activemq-artemis/pull/3644#issuecomment-875751977


   Might be worth adding a comment, and test, to avoid a recurrence.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] jbertram commented on pull request #3644: ARTEMIS-3327 Removing unecessary block oepration on journal append record

Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3644:
URL: https://github.com/apache/activemq-artemis/pull/3644#issuecomment-874997089


   My reading here is that the `result` wasn't actually used for anything so there was no reason to keep it. Is that how you see it?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3644: ARTEMIS-3327 Removing unecessary block oepration on journal append record

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3644:
URL: https://github.com/apache/activemq-artemis/pull/3644#issuecomment-874982143


   this is simply an optimization. There are no semantic changes on this commit.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org