You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/04/24 00:17:26 UTC
[GitHub] [accumulo] tushardhadiwal opened a new pull request #1594: Issue 1588 : Refine rename during minor compaction
tushardhadiwal opened a new pull request #1594:
URL: https://github.com/apache/accumulo/pull/1594
Issue 1588 : Refine rename during minor compaction
PR addresses a few issues:
Spurious rename failures where rename failed but actually succeeded
The check for file existence should only be done before attempting rename
Should not delete file if it exists (an admin may want to investigate).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] keith-turner commented on pull request #1594: Fix #1588 : Refine rename during minor compaction
Posted by GitBox <gi...@apache.org>.
keith-turner commented on pull request #1594:
URL: https://github.com/apache/accumulo/pull/1594#issuecomment-619163419
I think #1588 is a more general issue that could still be beneficial and be done at some point in the future. Could change the commit message for this PR to something like `Handle spurious failure were rename succeeded but reported it failed` and omit #1588 or just reference #1588 in the commit message without the preceding `fix`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] arvindshmicrosoft commented on a change in pull request #1594: Fix #1588 : Refine rename during minor compaction
Posted by GitBox <gi...@apache.org>.
arvindshmicrosoft commented on a change in pull request #1594:
URL: https://github.com/apache/accumulo/pull/1594#discussion_r414278456
##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java
##########
@@ -329,23 +329,34 @@ void bringMinorCompactionOnline(TabletFile tmpDatafile, TabletFile newDatafile,
StoredTabletFile newFile;
// rename before putting in metadata table, so files in metadata table should
// always exist
+ boolean attemptedRename = false;
do {
try {
if (dfv.getNumEntries() == 0) {
+
tablet.getTabletServer().getFileSystem().deleteRecursively(tmpDatafile.getPath());
- } else {
- if (tablet.getTabletServer().getFileSystem().exists(newDatafile.getPath())) {
+ } else // i.e. dfv.getNumEntries() > 0
+ {
Review comment:
@tushardhadiwal some minor comments:
* Please modify the placement of this curly brace to match the project's expected style, otherwise the CI fails. The [patch submission steps](https://github.com/apache/accumulo/blob/master/CONTRIBUTING.md#patch-submission) are very useful - ```mvn clean verify -DskipTests``` will usually auto-format code and fix this
* Related to above - personally I'd also remove the // i.e. dfv.getNumEntries() > 0 comment :-)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] tushardhadiwal commented on a change in pull request #1594: Handle spurious failure were rename succeeded but reported it failed (#1588)
Posted by GitBox <gi...@apache.org>.
tushardhadiwal commented on a change in pull request #1594:
URL: https://github.com/apache/accumulo/pull/1594#discussion_r414789906
##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java
##########
@@ -329,23 +329,34 @@ void bringMinorCompactionOnline(TabletFile tmpDatafile, TabletFile newDatafile,
StoredTabletFile newFile;
// rename before putting in metadata table, so files in metadata table should
// always exist
+ boolean attemptedRename = false;
do {
try {
if (dfv.getNumEntries() == 0) {
+
tablet.getTabletServer().getFileSystem().deleteRecursively(tmpDatafile.getPath());
- } else {
- if (tablet.getTabletServer().getFileSystem().exists(newDatafile.getPath())) {
+ } else // i.e. dfv.getNumEntries() > 0
+ {
Review comment:
@arvindshmicrosoft I have removed the comment & fixed the curly brace.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] keith-turner commented on pull request #1594: Handle spurious failure were rename succeeded but reported it failed (#1588)
Posted by GitBox <gi...@apache.org>.
keith-turner commented on pull request #1594:
URL: https://github.com/apache/accumulo/pull/1594#issuecomment-620698845
@tushardhadiwal thanks for the contribution. If you would like to be listed as a contributor on the [people page](https://accumulo.apache.org/people), please [submit a PR to add yourself](https://github.com/apache/accumulo-website/edit/master/pages/people.md). Let me know if you have any questions about editing the website.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org