wchevreuil commented on issue #740: HBASE-23197 'IllegalArgumentException: Wrong FS' on edits replay when… URL: https://github.com/apache/hbase/pull/740#issuecomment-554635904 > Chatted with wellington a little bit to give some more context (I was worried I would be too vague, but here goes anyways). Hey @joshelser , apologies, finally managed to "re-review" what I had thought here when proposed this solution. I had put some replies to your previous comments, let me know if that makes sense. Also had some more remarks below. > My concern stems from having multiple paths in which we call `HFileArchive.archiveStoreFiles` which have both the storefile FS and the wal FS passed in. Tried to map the possibilities on one of my replies above. As such, my concern is that the logic in this patch currently may still fall short: > > 1. We have a separate FS for WALs > 2. We have HFileArchiver.archiveStoreFiles is called with the rootdir's FS (via HRegion) > 3. We fail in the same manner See my scenario #4 on of above replies. This case would only happen when we archiving store files, so it would pick up the `rootdir` value. > > I don't have a specific codepath which I think will hit this problem (yet), but it worries me that we're leaving this gap. For example, HStore will call this method for the normal StoreFile archival path. I think it would make me more comfortable if we at least have a separate entry point to archive things on the WAL FS, even if it's just giving us a different method name to call and clear expectations on what FS (and the scheme which should be valid). I think keeping the same underlying implementation is fine (better to not copy that code). I understand the confusion, reusing the very same path is misleading. Maybe we can have this as a quick solution (provided we agree it's solving the issue without breaking current functionality), then we work on a larger refactoring on a separate jira? That would be simpler and quicker to backport for older branches.
---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services
