Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* 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
*
* http://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.hadoop.ozone.om;

import java.io.IOException;
import java.util.Map;

/**
* Interface to manage Ozone snapshot DB local checkpoint metadata properties.
* Those properties are per-OM, e.g. isSSTFiltered flag, which differs from the ones stored in SnapshotInfo proto.
*/
public interface OmSnapshotLocalProperty extends AutoCloseable {

/**
* Sets a property for a snapshot.
*
* @param key Property key
* @param value Property value
* @throws IOException if an I/O error occurs
*/
void setProperty(String key, String value) throws IOException;
Comment thread
smengcl marked this conversation as resolved.
Outdated

/**
* Gets a property value for a snapshot.
*
* @param key Property key
* @return Property value or null if not found
* @throws IOException if an I/O error occurs
*/
String getProperty(String key) throws IOException;
Comment thread
smengcl marked this conversation as resolved.
Outdated

/**
* Gets all properties for a snapshot.
*
* @return Map of property key-value pairs
* @throws IOException if an I/O error occurs
*/
Map<String, String> getProperties() throws IOException;

/**
* Checks if a property exists for a snapshot.
*
* @param key Property key
* @return true if the property exists, false otherwise
* @throws IOException if an I/O error occurs
*/
boolean hasProperty(String key) throws IOException;

/**
* Removes a property from a snapshot.
*
* @param key Property key
* @throws IOException if an I/O error occurs
*/
void removeProperty(String key) throws IOException;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
/*
* 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
*
* http://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.hadoop.ozone.om;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.hadoop.hdds.server.YamlUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.yaml.snakeyaml.DumperOptions;
import org.yaml.snakeyaml.Yaml;
import org.yaml.snakeyaml.scanner.ScannerException;

/**
* Implementation of {@link OmSnapshotLocalProperty} that uses a YAML file
* to store and retrieve snapshot local properties.
* Changes are only persisted when the object is closed.
*/
public class OmSnapshotLocalPropertyYamlImpl implements OmSnapshotLocalProperty, AutoCloseable {

private static final Logger LOG = LoggerFactory.getLogger(OmSnapshotLocalPropertyYamlImpl.class);

/**
* The path to the YAML file used to store properties.
* If this file does not exist, it will be created upon close().
*/
private final File yamlFile;

/**
* A map storing key-value pairs of snapshot properties.
* Read from the YAML file upon initialization.
*/
private Map<String, String> properties;
Comment thread
smengcl marked this conversation as resolved.
Outdated

/**
* Flag indicating whether the properties have been modified
* since the last save to the YAML file.
* Used to determine if file write is needed on close.
*/
private final AtomicBoolean isDirty = new AtomicBoolean(false);

/**
* Flag indicating whether this instance has been closed.
* Operations attempted after closing will throw an OMException.
*/
private final AtomicBoolean isClosed = new AtomicBoolean(false);

/**
* Constructs a new OmSnapshotLocalPropertyYamlImpl.
*
* @param yamlFilePath Path to the YAML file
*/
public OmSnapshotLocalPropertyYamlImpl(String yamlFilePath) throws IOException {
this.yamlFile = new File(yamlFilePath);
loadPropertiesFromFile();
}

@Override
public void setProperty(String key, String value) throws IOException {
Comment thread
smengcl marked this conversation as resolved.
Outdated
checkIfClosed();
String oldValue = properties.get(key);
if (!Objects.equals(value, oldValue)) {
properties.put(key, value);
isDirty.set(true);
}
}

@Override
public String getProperty(String key) throws IOException {
checkIfClosed();
return properties.get(key);
}

@Override
public boolean hasProperty(String key) throws IOException {
checkIfClosed();
return properties.containsKey(key);
}

@Override
public void removeProperty(String key) throws IOException {
checkIfClosed();
if (properties.containsKey(key)) {
properties.remove(key);
isDirty.set(true);
}
}

@Override
public Map<String, String> getProperties() throws IOException {
checkIfClosed();
return Collections.unmodifiableMap(properties);
}

/**
* Saves any pending changes to the YAML file and releases resources.
*
* @throws IOException if an I/O error occurs saving the file
*/
@Override
public void close() throws IOException {
if (isClosed.compareAndSet(false, true)) {
if (isDirty.get()) {
LOG.debug("Saving changes to properties file: {}", yamlFile);
savePropertiesToFile();
}
}
}

/**
* Checks if the object has been closed.
*
* @throws IOException if the object has been closed
*/
private void checkIfClosed() throws IOException {
if (isClosed.get()) {
throw new IOException("OmSnapshotLocalPropertyYamlImpl has been closed");
}
}

/**
* Loads the properties from the YAML file.
*
* @throws IOException if an I/O error occurs, or if the YAML file is not properly formatted
*/
private void loadPropertiesFromFile() throws IOException {
if (!yamlFile.exists()) {
LOG.debug("YAML file does not exist, creating empty properties map");
properties = new HashMap<>();
return;
}

try (InputStream inputStream = Files.newInputStream(yamlFile.toPath())) {
Map<String, String> loadedProperties = YamlUtils.loadAs(inputStream, Map.class);
properties = loadedProperties != null ? new HashMap<>(loadedProperties) : new HashMap<>();
} catch (IOException | ScannerException e) {
LOG.error("Unable to parse snapshot local properties YAML file: {}", yamlFile, e);
throw new IOException("Unable to parse snapshot local properties YAML file", e);
}
}

/**
* Saves the properties to the YAML file.
*
* @throws IOException if an I/O error occurs
*/
private void savePropertiesToFile() throws IOException {
DumperOptions options = new DumperOptions();
options.setPrettyFlow(true);
options.setDefaultFlowStyle(DumperOptions.FlowStyle.FLOW);
Yaml yaml = new Yaml(options);

YamlUtils.dump(yaml, properties, yamlFile, LOG);
isDirty.set(false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,11 @@ public static String getSnapshotPath(OzoneConfiguration conf,
OM_DB_NAME + snapshotInfo.getCheckpointDirName();
}

public static String getSnapshotLocalPropertyYamlPath(OzoneConfiguration conf,
SnapshotInfo snapshotInfo) {
return getSnapshotPath(conf, snapshotInfo) + ".yaml";
Comment thread
smengcl marked this conversation as resolved.
}

public static boolean isSnapshotKey(String[] keyParts) {
return (keyParts.length > 1) &&
(keyParts[0].compareTo(OM_SNAPSHOT_INDICATOR) == 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public class SstFilteringService extends BackgroundService
private static final int SST_FILTERING_CORE_POOL_SIZE = 1;

public static final String SST_FILTERED_FILE = "sstFiltered";
public static final String SST_FILTERED_YAML_KEY = "sstFiltered";
private static final byte[] SST_FILTERED_FILE_CONTENT = StringUtils.string2Bytes("This file holds information " +
"if a particular snapshot has filtered out the relevant sst files or not.\nDO NOT add, change or delete " +
"any files in this directory unless you know what you are doing.\n");
Expand All @@ -86,8 +87,20 @@ public class SstFilteringService extends BackgroundService
private final BootstrapStateHandler.Lock lock = new BootstrapStateHandler.Lock();

public static boolean isSstFiltered(OzoneConfiguration ozoneConfiguration, SnapshotInfo snapshotInfo) {
Path sstFilteredFile = Paths.get(OmSnapshotManager.getSnapshotPath(ozoneConfiguration,
snapshotInfo), SST_FILTERED_FILE);
// First try to read the flag from YAML file
String yamlPath = OmSnapshotManager.getSnapshotLocalPropertyYamlPath(ozoneConfiguration, snapshotInfo);

try (OmSnapshotLocalProperty localProperties = new OmSnapshotLocalPropertyYamlImpl(yamlPath)) {

@swamirishi swamirishi Jun 9, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There could be a potential race condition if multiple threads parallely try to open. It would be better if this comes from snapshotCache or make the cache abstract (ReferenceCountedCache) enough to load any resource and have one implementation for loading OmSnapshot and one reference counted cache implementation for OmSnapshotLocalProperty

@swamirishi swamirishi Jun 9, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OmSnapshotLocalProperty can also take a write lock on the snapshotId to prevent SNAPSHOT_PROPERTY_LOCK from getting updated by multiple threads. This could be simpler.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. If we want it to be race-free, an easy thought would be to guard r/w access behind SnapshotCache r/w lock. But one thing I don't like is that it implies we are opening the snapshot DB just for reading the yaml metadata (assuming no major refactoring of the SnapshotCache).

Let me see if flock would work..

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah lock would be simpler. We can have duplicate objects in that case

@SaketaChalamchala SaketaChalamchala Jun 10, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would also need to take a bootstrap lock here

BootstrapStateHandler.java

@swamirishi swamirishi Jun 10, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might not need bootstrap handler lock. Since we would be taking a lock on all the background services which would be updating the metadata file and no ratis transaction excepting for create snapshot is going to update this metadata file.

String sstFilteredProperty = localProperties.getProperty(SST_FILTERED_YAML_KEY);
return Boolean.parseBoolean(sstFilteredProperty);
} catch (Exception e) {
// If we can't read the YAML file, fall back to the existing checks
LOG.debug("Failed to read snapshot local properties from YAML file: {}", yamlPath, e);
}

// Fall back to existing checks
Path sstFilteredFile = Paths.get(
OmSnapshotManager.getSnapshotPath(ozoneConfiguration, snapshotInfo), SST_FILTERED_FILE);
return snapshotInfo.isSstFiltered() || sstFilteredFile.toFile().exists();
}

Expand Down Expand Up @@ -119,14 +132,14 @@ public void resume() {
running.set(true);
}

private class SstFilteringTask implements BackgroundTask {
private final class SstFilteringTask implements BackgroundTask {

private boolean isSnapshotDeleted(SnapshotInfo snapshotInfo) {
return snapshotInfo == null || snapshotInfo.getSnapshotStatus() == SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED;
}

/**
* Marks the snapshot as SSTFiltered by creating a file in snapshot directory.
* Marks the snapshot as SSTFiltered.
* @param snapshotInfo snapshotInfo
* @throws IOException
*/
Expand All @@ -141,8 +154,18 @@ private void markSSTFilteredFlagForSnapshot(SnapshotInfo snapshotInfo) throws IO
if (acquiredSnapshotLock) {
String snapshotDir = OmSnapshotManager.getSnapshotPath(ozoneManager.getConfiguration(), snapshotInfo);
try {
// mark the snapshot as filtered by creating a file.
// Mark the snapshot as filtered by writing to YAML property file
if (Files.exists(Paths.get(snapshotDir))) {
String yamlPath = OmSnapshotManager.getSnapshotLocalPropertyYamlPath(
ozoneManager.getConfiguration(), snapshotInfo);
try (OmSnapshotLocalProperty localProperties = new OmSnapshotLocalPropertyYamlImpl(yamlPath)) {
localProperties.setProperty(SST_FILTERED_YAML_KEY, "true");
} catch (Exception e) {
LOG.error("Failed to set SST filtered local property for snapshot: {}", snapshotInfo.getName(), e);
}

// For backward compatibility, still create the touch file (e.g. when upgraded but not finalized yet)
// TODO: When upgrade is finalized, this can be skipped
Files.write(Paths.get(snapshotDir, SST_FILTERED_FILE), SST_FILTERED_FILE_CONTENT);
}
} finally {
Expand Down
Loading