Uh oh!
There was an error while loading. Please reload this page.
HBASE-24086 Disable output stream capability enforcement when running in standalone mode - #1408
Conversation
Apache-HBase
commented
Apr 1, 2020
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Apr 1, 2020
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Apr 1, 2020
🎊 +1 overall
This message was automatically generated. |
bharathv
left a comment
There was a problem hiding this comment.
+1 for better out of the box defaults..
| if (enforceStreamCapability != null) { | ||
| fs.getConf().set(UNSAFE_STREAM_CAPABILITY_ENFORCE, enforceStreamCapability); | ||
| } | ||
| if (!c.getBoolean(HConstants.CLUSTER_DISTRIBUTED, false)) { |
There was a problem hiding this comment.
nit: make it an else if? meaning if the user didn't override UNSAFE_STREAM_CAPABILITY_ENFORCE and its a local fs mode..
There was a problem hiding this comment.
Can you not ask fs if it is an instance of local fs? If it is, then set the boolean?
There was a problem hiding this comment.
Hmm.... Looking here, the distributed flag is good to check [1] but would we run non-distributed on an fs that supports sync? s3?
There was a problem hiding this comment.
make it an else if? meaning if the user didn't override UNSAFE_STREAM_CAPABILITY_ENFORCE and its a local fs mode..
I explicitly disable it here because we know it doesn't work with the local filesystem. There are plenty of places where conf lookups of UNSAFE_STREAM_CAPABILITY_ENFORCE have a default of true, so leaving it unset sometimes doesn't help.
There was a problem hiding this comment.
Can you not ask fs if it is an instance of local fs? If it is, then set the boolean?
would we run non-distributed on an fs that supports sync? s3?
My preference would be if !distributed && !fs.supportsCapabilities(), however the only way I see to ask a FS if it supports the capabilities is to first instantiate a stream, which I didn't want to do here. Other suggestions?
There was a problem hiding this comment.
I think @saintstack is right. We can run non-distributed on a distributed fs with stream capabilities. So the check is probably not tight...
Other suggestions?
One naive way is to just check for instance of LocalFileSystem and override? (Think this is what Stack was suggesting). This covers the most common case of testing something by just unpacking a bin release. Any other fancy filesystems, users may have to figure out themselves?
There was a problem hiding this comment.
I was hoping to avoid pinning the logic to specific FileSystem implementations, rather on capabilities. Fun fact: the only commonality between LocalFileSystem and RawLocalFileSystem is FileSystem :/
There was a problem hiding this comment.
Ya, that'd be ideal.. probably a new Hadoop jira for the FileSystem interface to expose the capabilities in some form...
There was a problem hiding this comment.
From debugging, it's LocalFileSystem that gets used for wals, so that's what I've used. This seems very fragile though :/
Apache-HBase
commented
Apr 2, 2020
🎊 +1 overall
This message was automatically generated. |
saintstack
left a comment
There was a problem hiding this comment.
I think this better.
(I thought the capabilities a new feature added in 3.0.0 because the likes of EC changed fundamental behavior -- throwing an exception where before it would just continue if it didn't support sync -- so am a little wary relying on it; i could be way wrong).
| if (enforceStreamCapability != null) { | ||
| fs.getConf().set(UNSAFE_STREAM_CAPABILITY_ENFORCE, enforceStreamCapability); | ||
| } | ||
| if (!c.getBoolean(HConstants.CLUSTER_DISTRIBUTED, false) |
There was a problem hiding this comment.
Just test whether this is a LocalFileSystem is enough?
There was a problem hiding this comment.
You tell me :)
Yes, for running out of the source tree, not modifying any configuration, this change permits the process to start, master and region server threads all come online. I performed some additional verification by running ltt against the process, a scant 100k rows.
I don't know well when we choose LocalFileSystem vs. RawLocalFIleSystem, so there may be other issues lurking.
There was a problem hiding this comment.
Hmm.. I too think it should just be (fs instanceof LocalFS).. With this patch, try using the following combination (distributed cluster + localfs), it fails with the same error when it should work..
<configuration>
<property>
<name>hbase.cluster.distributed</name>
<value>true</value>
</property>
</configuration>
Point being that, these two configuration params are disjoint. Meaning a distributed/non-distributed cluster should work with a local file system. Stream capability is a property of the FS in use, so just a check on FS should be good IMO.
I poked around a little bit about RawFS in the hadoop code, didn't fully understand its use but it appears that FileSystem when parsing the config would pick LocalFileSystem when the scheme is file:///. So I think just doing it for LocalFileSystem should be ok.
There was a problem hiding this comment.
How do you run a distributed cluster on local fs? You running off of shared NFS mounts?
There was a problem hiding this comment.
Pseudo-distributed http://hbase.apache.org/book.html#quickstart_pseudo, just that I didn't set "hbase.rootdir" and hence it defaults to local fs.
There was a problem hiding this comment.
Yeah, fair enough. So I'll remove the CLUSTER_DISTRIBUTED check and instead check fs instanceOf LocalFileSystem.
I also asked the stream capabilities question over on hdfs-user.
Apache-HBase
commented
Apr 2, 2020
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Apr 2, 2020
🎊 +1 overall
This message was automatically generated. |
8d798e7 to
cbe08c6Comparendimiduk
commented
Apr 2, 2020
|
bharathv
left a comment
There was a problem hiding this comment.
Might need some doc changes to the quick start page.. probably can be done as a follow up.
ndimiduk
commented
Apr 2, 2020
Sure, I created HBASE-24106 so we can hash out the docs there. |
Apache-HBase
commented
Apr 2, 2020
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Apr 2, 2020
💔 -1 overall
This message was automatically generated. |
Apache-HBase
commented
Apr 2, 2020
🎊 +1 overall
This message was automatically generated. |
… on LocalFileSystem Signed-off-by: stack <stack@apache.org> Signed-off-by: Bharath Vissapragada <bharathv@apache.org> Signed-off-by: Duo Zhang <zhangduo@apache.org>
cbe08c6 to
40b316fComparendimiduk
commented
Apr 3, 2020
Thanks for the thoughtful reviews. |
No description provided.