You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2022/02/07 18:32:14 UTC
[GitHub] [geode] albertogpz opened a new pull request #7345: GEODE-10002: Document recommendation but not obligation for parallel …
albertogpz opened a new pull request #7345:
URL: https://github.com/apache/geode/pull/7345
…gateway sender to share disk store with region
<!-- Thank you for submitting a contribution to Apache Geode. -->
<!-- In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
-->
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [ ] Is your initial contribution a single, squashed commit?
- [ ] Does `gradlew build` run cleanly?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
<!-- Note:
Please ensure that once the PR is submitted, check Concourse for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.
-->
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] albertogpz commented on pull request #7345: GEODE-10002: Document recommendation but not obligation for parallel …
Posted by GitBox <gi...@apache.org>.
albertogpz commented on pull request #7345:
URL: https://github.com/apache/geode/pull/7345#issuecomment-1032445932
I'd like @upthewaterspout and @karensmolermiller to confirm if the change proposed is ok or if we should even go further and remove the recommendation.
I do not know what was the origin for the previous ticket and PR that provoked the addition of the obligation to have the same disk-store (maybe an operation error in a customer that deleted the files). In my opinion, to prevent a hang in the startup because someone could delete manually some files is not a reason to even recommend this.
I see advantages in separating the disk-stores of the data region from the queue events and that's why I am reluctant to even have the recommendation if the only reason is the one above.
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] albertogpz commented on pull request #7345: GEODE-10002: Remove in documentation obligation for parallel …
Posted by GitBox <gi...@apache.org>.
albertogpz commented on pull request #7345:
URL: https://github.com/apache/geode/pull/7345#issuecomment-1041312344
> As per https://cwiki.apache.org/confluence/display/GEODE/Commit+Message+Format, first commit summary should be under 29 characters (not including bug# or pr#) (yours is 102 characters) and details should explain what the intent of the commit is and why. If you amend your initial commit now and force-push, you can benefit from review feedback on your commit message, and you won't have to remember to fix it later when you merge.
@onichols-pivotal Can I still do that even if the pull request is already merged and closed?
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] onichols-pivotal commented on pull request #7345: GEODE-10002: Remove in documentation obligation for parallel …
Posted by GitBox <gi...@apache.org>.
onichols-pivotal commented on pull request #7345:
URL: https://github.com/apache/geode/pull/7345#issuecomment-1041495218
Oops, I didn't see this was already merged.
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] albertogpz merged pull request #7345: GEODE-10002: Remove in documentation obligation for parallel …
Posted by GitBox <gi...@apache.org>.
albertogpz merged pull request #7345:
URL: https://github.com/apache/geode/pull/7345
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] albertogpz commented on pull request #7345: GEODE-10002: Document recommendation but not obligation for parallel …
Posted by GitBox <gi...@apache.org>.
albertogpz commented on pull request #7345:
URL: https://github.com/apache/geode/pull/7345#issuecomment-1038742735
> I like this change. I would be ok with going further and removing the restriction. As far as I know the only reason to put the region and the sender in the same disk store is to avoid accidentally deleting one of the disk stores.
>
> A long time ago we discussed some changes that might make it better for a region and a sender to be the same disk store. For example, the new value in the region and the new value going to the gateway sender are the same bytes, so we might theoretically be able to to avoid duplicating those bytes on disk. However, we're not doing anything like that right now.
@upthewaterspout Thanks for your answer. I think it is best to remove the restriction given that the only reason we have for it is to avoid problems when there is an accidental deletion of one of the disk stores.
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org