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/06/03 18:57:39 UTC
[GitHub] [accumulo] milleruntime opened a new issue #1620: Possible loss of parallelism in VolumeImpl
milleruntime opened a new issue #1620:
URL: https://github.com/apache/accumulo/issues/1620
The implementation of VolumeManager may be negating some of the work done in other areas of code where we are using threadpools and futures to improve calls to the Namenode. I think this is only noticeable during the Bulk import processes (old and new) when we are calling [VolumeManager.rename()](https://github.com/apache/accumulo/blob/dd4a488a9077cdd5e90cbff89fbcf7e9147381a7/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java#L282) when moving the files.
@ivakegg and I recently saw a lot of threads waiting on a single rename during a Bulk import in 1.9. It is possible the way that we are reusing the hadoop FileSystem in the VolumeManager code in 1.9 is causing this. It is also possible that this was fixed with the refactor done in #1553
----------------------------------------------------------------
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] milleruntime commented on issue #1620: Possible loss of parallelism in VolumeImpl
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #1620:
URL: https://github.com/apache/accumulo/issues/1620#issuecomment-645459605
> There is a possibly related change for import table #1569
>
> I think it would be nice to have a VolumeManager.bulkRename() method for renaming lots of files. Then we can have a single implementation of this in the code to fix and maintain.
I started looking into this and noticed some subtle differences in debugging and exception handling betwen tableImport and bulk import use of the thread pool. I am not sure if the behavior is specific to the actual FATE operation or it is just how the code evolved independently. For example, BulkImportMove has this [check](https://github.com/apache/accumulo/blob/15a95feebc74bd10e5ba73d4c2ecfc726435bc55/server/master/src/main/java/org/apache/accumulo/master/tableOps/bulkVer2/BulkImportMove.java#L131) for existence while the [MoveExportedFiles](https://github.com/apache/accumulo/blob/27acec198401942fbd5d90edfea71ac859533470/server/master/src/main/java/org/apache/accumulo/master/tableOps/tableImport/MoveExportedFiles.java#L117) does not.
----------------------------------------------------------------
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 issue #1620: Possible loss of parallelism in VolumeImpl
Posted by GitBox <gi...@apache.org>.
keith-turner commented on issue #1620:
URL: https://github.com/apache/accumulo/issues/1620#issuecomment-646311069
> For example, BulkImportMove has this check for existence while the MoveExportedFiles does not.
That is an interesting difference. Both are FATE operations so they both need to be able to handle running part of the operation, process dying, and then having to run the move operation again later. Would be nice if they used the same code for this that could possibly be tested.
----------------------------------------------------------------
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] milleruntime commented on issue #1620: Possible loss of parallelism in VolumeImpl
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #1620:
URL: https://github.com/apache/accumulo/issues/1620#issuecomment-646617405
> > For example, BulkImportMove has this check for existence while the MoveExportedFiles does not.
>
> That is an interesting difference. Both are FATE operations so they both need to be able to handle running part of the operation, process dying, and then having to run the move operation again later. Would be nice if they used the same code for this that could possibly be tested.
That is what I was thinking too. I have a branch where I basically combined the features of both that I will put a pull request up soon.
----------------------------------------------------------------
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 issue #1620: Possible loss of parallelism in VolumeImpl
Posted by GitBox <gi...@apache.org>.
keith-turner commented on issue #1620:
URL: https://github.com/apache/accumulo/issues/1620#issuecomment-638415810
There is a possibly related change for import table #1569
I think it would be nice to have a VolumeManager.bulkRename() method for renaming lots of files. Then we can have a single implementation of this in the code to fix and maintain.
----------------------------------------------------------------
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] milleruntime commented on issue #1620: Possible loss of parallelism in VolumeImpl
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #1620:
URL: https://github.com/apache/accumulo/issues/1620#issuecomment-638431550
It could also be that Hadoop is returning the same object either way since it caches FileSystem objects on the static FileSystem get(uri, conf) call. Sorry can't find a link to the source code in Hadoop :roll_eyes:
The code I am looking at will always return this:
```return CACHE.get(uri, conf);```
----------------------------------------------------------------
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