dlmarion commented on code in PR #5458:
URL: https://github.com/apache/accumulo/pull/5458#discussion_r2038275844
##########
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManager.java:
##########
@@ -207,28 +208,51 @@ default Volume getFirst() {
Logger log = LoggerFactory.getLogger(VolumeManager.class);
- static InstanceId getInstanceIDFromHdfs(Path instanceDirectory,
Configuration hadoopConf) {
+ static InstanceInfo getInstanceInfoFromHdfs(Path instanceDirectory,
Configuration hadoopConf) {
try {
- FileSystem fs =
- VolumeConfiguration.fileSystemForPath(instanceDirectory.toString(),
hadoopConf);
+ log.debug("Trying to read instance info from {}", instanceDirectory);
+ var fs =
VolumeConfiguration.fileSystemForPath(instanceDirectory.toString(), hadoopConf);
FileStatus[] files = null;
try {
files = fs.listStatus(instanceDirectory);
} catch (FileNotFoundException ex) {
// ignored
}
- log.debug("Trying to read instance id from {}", instanceDirectory);
- if (files == null || files.length == 0) {
- log.error("unable to obtain instance id at {}", instanceDirectory);
- throw new IllegalStateException(
- "Accumulo not initialized, there is no instance id at " +
instanceDirectory);
- } else if (files.length != 1) {
- log.error("multiple potential instances in {}", instanceDirectory);
+ InstanceId instanceId = null;
Review Comment:
I think this changed code could be greatly simplified if the instance and
name were serialized as json into one file that has a name that we are
expecting. I think InstanceInfo can just be serialized and deserialized using
Gson in this case.
Having one file instead of a prefix negates the need to handle multiple
possible files for the name and id. Also, having one file means we are only
reading one file from HDFS instead of N.
##########
core/src/main/java/org/apache/accumulo/core/dataImpl/InstanceInfo.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.core.dataImpl;
+
+import static java.util.Objects.requireNonNull;
+
+import java.util.Objects;
+
+import org.apache.accumulo.core.data.InstanceId;
+
+public class InstanceInfo {
+
+ private final String name;
+ private final InstanceId id;
Review Comment:
I made a comment elsewhere about just serializing and deserializing this
object using a single file in HDFS in a known location. If we do that, then I
think we can collapse the instance version into this object as well and get rid
of that file. It may also make sense to add a version number to this object to
handle changes over time, much like a serialVersionUid.
--
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.
To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
