[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-388280851 Recap: @sijie reviewed and approved @jvrao reviewed and approved, suggesting to change the name from sync to force, but without being -1 I will wait untill Monday and than move forward. We can change the name with a follow up change, we are not touching raw wire protocol, it is only a matter of variables and method names not exposed to the public Thank you all guys for helping with this important change This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-388279709 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-388279689 jenkins had problems on disk, re-kicking tests This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387964825 @sijie I will stay tuned. I apologize to be annoying on this topic. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387962407 @sijie @jvrao since these "names" (forceLedger vs sync) are only on bare code, not on public client API and not on wire protocol, I would like to merge this patch now so that bookie side changes are totally in this way I can move forwards with steps on client: we cannot merge stuff on client if server side stuff is not complete I can send a follow up change to rename variables and methods if we feel strong about changing *force* to *sync*. Do this sound reasonable to you ? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side
implementation
URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387961769
I kicked tests again, the error seemed not related to the patch ("no enough
native threads"...)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387961684 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387775537 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387687819 retest this please This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387640856 @jvrao I have updated the patch with the assertions, let's wait for Sijie's (and eventually Ivan) final comments before merging. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387634874 @jvrao the name force superseded "sync" after a quite long discussion between JV,Sijie,Ivan and me and we agreed on that final name. The motivation is that Java APIs use the 'force' verb https://docs.oracle.com/javase/7/docs/api/java/nio/channels/FileChannel.html#force(boolean) on linux we have *fsync*/*fdatasync*, I don't know the windows world but with a quick search I found *fflush*, so using the Java name is more neutral I am open to discussion and decide a new name, from my point of view (coming from Java world and Linux world) every of the two names make sense. So @sijie and @ivankelly were mostly involved in that discussion I I think we need to hear their voices if you feel strong about changing this name. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387634874 @jvrao the name force superseded "sync" after a quite long discussion between JV,Sijie,Ivan and me and we agreed on that final name. The motivation is that Java APIs use the 'force' verb https://docs.oracle.com/javase/7/docs/api/java/nio/channels/FileChannel.html#force(boolean) on linux we have *fsync*/*fdatasync*, I don't know the windows world but with a quick search I found *fflush*, so using the Java name is more neutral I am open to discussion and decide a new name, from my point of view (coming from Java world and Linux world) every of the two names make sense. So @sijie and @ivankelly were mostly involved in that discussion I I think we need to hear their voices I you feel strong about changing this name. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387345829 @sijie @jvrao rebased to current master and ready for final review This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387345829 @sijie @jvrao reabase to current master and ready for final review This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-387343131 @sijie I saw your commit, I am just rebasing right now :-) thank you This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-386875224 @sijie @jvrao checkstyle fixed, this patch is ready for review. I am working to client side stuff now This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-386603080 Note for reviewer: please take into account only commit https://github.com/apache/bookkeeper/pull/1393/commits/ab50a51ffbdab4312765115ffdfe7fcf71184088 this branch is based on #1375 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-386735159 Checkstyle is not passing. I will fix soon. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation
eolivelli commented on issue #1393: BP-14 forceLedger wire protocol server side implementation URL: https://github.com/apache/bookkeeper/pull/1393#issuecomment-386603080 Note for reviewer: please take into account only commit https://github.com/apache/bookkeeper/pull/1393/commits/9dcefd6284dd6b2304b6abfdc6598e90d1bb71a9 this branch is based on #1375 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
