Apache9 commented on a change in pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#discussion_r478455231



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
##########
@@ -290,35 +272,48 @@ public TableDescriptor get(final TableName tablename)
     * @see #get(org.apache.hadoop.hbase.TableName)
     */
   @Override
-  public Map<String, TableDescriptor> getByNamespace(String name)
-  throws IOException {
+  public Map<String, TableDescriptor> getByNamespace(String name) throws 
IOException {
     Map<String, TableDescriptor> htds = new TreeMap<>();
     List<Path> tableDirs =
-        FSUtils.getLocalTableDirs(fs, CommonFSUtils.getNamespaceDir(rootdir, 
name));
-    for (Path d: tableDirs) {
-      TableDescriptor htd = null;
-      try {
-        htd = get(CommonFSUtils.getTableName(d));
-      } catch (FileNotFoundException fnfe) {
-        // inability of retrieving one HTD shouldn't stop getting the remaining
-        LOG.warn("Trouble retrieving htd", fnfe);
+      FSUtils.getLocalTableDirs(fs, CommonFSUtils.getNamespaceDir(rootdir, 
name));
+    for (Path d : tableDirs) {
+      TableDescriptor htd = get(CommonFSUtils.getTableName(d));
+      if (htd == null) {
+        continue;
       }
-      if (htd == null) continue;
       htds.put(CommonFSUtils.getTableName(d).getNameAsString(), htd);
     }
     return htds;
   }
 
-  /**
-   * Adds (or updates) the table descriptor to the FileSystem
-   * and updates the local cache with it.
-   */
   @Override
-  public void update(TableDescriptor htd) throws IOException {
+  public void update(TableDescriptor td, boolean cacheOnly) throws IOException 
{
+    // TODO: in fact this method will only be called at master side, so 
fsreadonly and usecache will
+    // always be true. In general, we'd better have a 
ReadOnlyFSTableDesciptors for HRegionServer
+    // but now, HMaster extends HRegionServer, so unless making use of 
generic, we can not have
+    // different implementations for HMaster and HRegionServer. Revisit this 
when we make HMaster
+    // not extend HRegionServer in the future.
     if (fsreadonly) {
-      throw new NotImplementedException("Cannot add a table descriptor - in 
read only mode");
+      throw new UnsupportedOperationException("Cannot add a table descriptor - 
in read only mode");
+    }
+    if (!cacheOnly) {
+      updateTableDesciptor(td);
+    }
+    if (usecache) {
+      this.cache.put(td.getTableName(), td);
     }
-    updateTableDescriptor(htd);
+  }
+
+  @VisibleForTesting
+  Path updateTableDesciptor(TableDescriptor td) throws IOException {

Review comment:
       Done.




----------------------------------------------------------------
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:
[email protected]


Reply via email to