Skip to content
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
c38ead2
added ops per sec and average pagination metric for listkeys
Dec 7, 2023
36ca4fa
updated test cases
Dec 12, 2023
1818a49
Updated all test cases
Dec 13, 2023
f731a19
refactored code
Dec 13, 2023
6d3d979
refactored code
Dec 13, 2023
18e1e00
fixed errors
Dec 14, 2023
6744486
fixed import errors
Dec 14, 2023
a8c1c0a
Merge branch 'master' into HDDS-9874
muskan1012 Dec 18, 2023
3375fce
rearranged imports and refactored code
muskan1012 Dec 18, 2023
68b8238
Merge branch 'master' into HDDS-9874
muskan1012 Jan 4, 2024
e879fc9
removed duplicate code
muskan1012 Jan 8, 2024
94545b6
code change to avoid build failure
muskan1012 Jan 10, 2024
860247b
Merge branch 'master' into HDDS-9874
muskan1012 Jan 28, 2024
3c15956
fixed test cases
muskan1012 Feb 7, 2024
2a3a512
Merge branch 'HDDS-9874' of github.com:muskan1012/ozone into HDDS-9874
muskan1012 Feb 7, 2024
7fb39c6
Merge branch 'apache:master' into HDDS-9874
muskan1012 Feb 7, 2024
1bd6fd7
fixed checkstyle and other issues
muskan1012 Feb 7, 2024
31f8a3c
checkstyle issue resolved
muskan1012 Feb 7, 2024
3a73859
unit test resolution
muskan1012 Feb 7, 2024
6bf2a5e
unit test case failures resolutions
muskan1012 Feb 8, 2024
8671712
listKeysReadFromRocksDbLatency metric added
muskan1012 Feb 8, 2024
a0397eb
removed irrelevant code
muskan1012 Feb 8, 2024
dfd427b
refactored code to avoid null pointer exception
muskan1012 Feb 8, 2024
cf5f43d
Merge branch 'master' into HDDS-9874
muskan1012 Mar 26, 2024
570bc36
Merge branch 'master' into HDDS-9874
muskan1012 Jun 18, 2024
364de9b
updated code to avoid npe
muskan1012 Jun 18, 2024
d26e986
npe resolution for integration-recon
muskan1012 Jun 18, 2024
2bb0cda
refactored code as per review comments
muskan1012 Jun 24, 2024
bf9d575
minor nits fixed
muskan1012 Jun 25, 2024
b0a5832
changes as per the review comment
muskan1012 Jul 1, 2024
1c83093
Apply suggestions from code review
muskan1012 Jul 1, 2024
8139ae7
changed naming convention as per the review comment
muskan1012 Jul 8, 2024
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
Expand Up @@ -29,6 +29,7 @@
import org.apache.hadoop.ozone.TestDataUtil;
import org.apache.hadoop.ozone.client.OzoneClient;
import org.apache.hadoop.ozone.debug.PrefixParser;
import org.apache.hadoop.ozone.om.OMPerformanceMetrics;
import org.apache.hadoop.ozone.om.OMStorage;
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
import org.junit.jupiter.api.AfterAll;
Expand Down Expand Up @@ -60,13 +61,15 @@ public class TestOzoneFileSystemPrefixParser {
private static Path dir;
private static Path file;

private static OMPerformanceMetrics perfMetrics;

@BeforeAll
public static void init() throws Exception {
volumeName = RandomStringUtils.randomAlphabetic(10).toLowerCase();
bucketName = RandomStringUtils.randomAlphabetic(10).toLowerCase();

configuration = new OzoneConfiguration();

perfMetrics = new OMPerformanceMetrics();
cluster = MiniOzoneCluster.newBuilder(configuration)
.setNumDatanodes(3)
.build();
Expand Down Expand Up @@ -130,23 +133,23 @@ private void testPrefixParseWithInvalidPaths() throws Exception {
RandomStringUtils.randomAlphabetic(10).toLowerCase();
invalidVolumeParser.parse(invalidVolumeName, bucketName,
OMStorage.getOmDbDir(configuration).getPath(),
file.toString());
file.toString(), perfMetrics);
assertPrefixStats(invalidVolumeParser, 0, 0, 0, 0, 0, 0);

PrefixParser invalidBucketParser = new PrefixParser();
String invalidBucketName =
RandomStringUtils.randomAlphabetic(10).toLowerCase();
invalidBucketParser.parse(volumeName, invalidBucketName,
OMStorage.getOmDbDir(configuration).getPath(),
file.toString());
file.toString(), perfMetrics);
assertPrefixStats(invalidBucketParser, 1, 0, 0, 0, 0, 0);


Path invalidIntermediateDir = new Path(file.getParent(), "xyz");
PrefixParser invalidIntermediateDirParser = new PrefixParser();
invalidIntermediateDirParser.parse(volumeName, bucketName,
OMStorage.getOmDbDir(configuration).getPath(),
invalidIntermediateDir.toString());
invalidIntermediateDir.toString(), perfMetrics);

assertPrefixStats(invalidIntermediateDirParser, 1, 1, 3, 1, 1, 1);

Expand All @@ -157,7 +160,7 @@ private void verifyPrefixParsePath(Path parent, int intermediateDirCount,
PrefixParser parser = new PrefixParser();

parser.parse(volumeName, bucketName,
OMStorage.getOmDbDir(configuration).getPath(), parent.toString());
OMStorage.getOmDbDir(configuration).getPath(), parent.toString(), perfMetrics);

assertPrefixStats(parser, 1, 1, intermediateDirCount, nonExistentDirCount,
fileCount, dirCount);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.apache.hadoop.ozone.client.OzoneClientFactory;
import org.apache.hadoop.ozone.client.OzoneVolume;
import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
import org.apache.hadoop.ozone.om.OMPerformanceMetrics;
import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
import org.apache.ozone.test.GenericTestUtils;
Expand Down Expand Up @@ -120,7 +121,7 @@ public static void init() throws Exception {
public void testContainerMapper() throws Exception {
ContainerMapper containerMapper = new ContainerMapper();
Map<Long, List<Map<Long, BlockIdDetails>>> dataMap =
containerMapper.parseOmDB(conf);
containerMapper.parseOmDB(conf, new OMPerformanceMetrics());
// As we have created 20 keys with 10 MB size, and each
// container max size is 100 MB, it should create 3 containers because
// containers are closing before reaching the threshold
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,21 @@ public static void unregister() {
@Metric(about = "Ratis local command execution latency in nano seconds")
private MutableRate validateAndUpdateCacheLatencyNs;

@Metric(about = "average pagination for listKeys")
private MutableRate listKeysAveragePagination;

@Metric(about = "ops per second for listKeys")
private MutableRate listKeysOpsPerSec;

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 have the Latency metric listKeysLatencyNs, maybe we just need to add a cumulative number of listKeys metric(such as: listKeysSize), which we can calculate to get listKeysOpsPerSec and listKeysAveragePagination.

@Metric(about = "listKeys latency in nanoseconds")
private MutableRate listKeysLatencyNs;

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.

I implemented the cumulative number of listKeys using keyCount, which is set to maxKeys if truncated or cacheKeyMap.size() otherwise.
listKeysAveragePagination is set using keyCount.
listKeysOpsPerSec is calculated as keyCount / ((Time.monotonicNowNanos() - startNanos) / 1_000_000_000.0f).
Added latency tracking with addListKeysReadFromRocksDbLatencyNs.
This approach captures the metrics as you suggested.


@Metric(about = "ACLs check latency in listKeys")
private MutableRate listKeysAclCheckLatencyNs;

@Metric(about = "resolveBucketLink latency in listKeys")
private MutableRate listKeysResolveBucketLatencyNs;

@Metric(about = "readFromRockDb latency in listKeys")
private MutableRate listKeysReadFromRocksDbLatencyNs;

public void addLookupLatency(long latencyInNs) {
lookupLatencyNs.add(latencyInNs);
}
Expand Down Expand Up @@ -216,11 +225,23 @@ public MutableRate getValidateAndUpdateCacheLatencyNs() {
return validateAndUpdateCacheLatencyNs;
}

public void setListKeysAveragePagination(long averagePagination) {
listKeysAveragePagination.add(averagePagination);
}

public void setListKeysOpsPerSec(long opsPerSec) {
listKeysOpsPerSec.add(opsPerSec);
}

MutableRate getListKeysAclCheckLatencyNs() {
return listKeysAclCheckLatencyNs;
}

MutableRate getListKeysResolveBucketLatencyNs() {
return listKeysResolveBucketLatencyNs;
}

public void addListKeysReadFromRocksDbLatencyNs(long latencyInNs) {
listKeysReadFromRocksDbLatencyNs.add(latencyInNs);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ public class OmMetadataManagerImpl implements OMMetadataManager,
private final Map<String, TableCacheMetrics> tableCacheMetricsMap =
new HashMap<>();
private SnapshotChainManager snapshotChainManager;
private final OMPerformanceMetrics perfMetrics;
private final S3Batcher s3Batcher = new S3SecretBatcher();

/**
Expand All @@ -327,28 +328,46 @@ public class OmMetadataManagerImpl implements OMMetadataManager,
*/
public OmMetadataManagerImpl(OzoneConfiguration conf,
OzoneManager ozoneManager) throws IOException {
this.ozoneManager = ozoneManager;
init(conf, ozoneManager);
this.lock = new OzoneManagerLock(conf);
this.omEpoch = OmUtils.getOMEpoch(isRatisEnabled);
this.perfMetrics = ozoneManager.getPerfMetrics();
Comment thread
muskan1012 marked this conversation as resolved.
Outdated
start(conf);
}

public OmMetadataManagerImpl(OzoneConfiguration conf,
OzoneManager ozoneManager,
OMPerformanceMetrics perfMetrics)
throws IOException {
init(conf, ozoneManager);
Comment thread
tanvipenumudy marked this conversation as resolved.
Outdated
this.lock = new OzoneManagerLock(conf);
// TODO: This is a temporary check. Once fully implemented, all OM state
// change should go through Ratis - be it standalone (for non-HA) or
// replicated (for HA).
isRatisEnabled = conf.getBoolean(
OMConfigKeys.OZONE_OM_RATIS_ENABLE_KEY,
OMConfigKeys.OZONE_OM_RATIS_ENABLE_DEFAULT);
this.omEpoch = OmUtils.getOMEpoch(isRatisEnabled);
this.perfMetrics = perfMetrics;
// For test purpose only
ignorePipelineinKey = conf.getBoolean(
"ozone.om.ignore.pipeline", Boolean.TRUE);
"ozone.om.ignore.pipeline", Boolean.TRUE);
start(conf);
}
Comment thread
muskan1012 marked this conversation as resolved.

private void init(OzoneConfiguration conf, OzoneManager manager) {
this.ozoneManager = manager;
// TODO: This is a temporary check. Once fully implemented, all OM state
// change should go through Ratis - be it standalone (for non-HA) or
// replicated (for HA).
Comment thread
tanvipenumudy marked this conversation as resolved.
Outdated
isRatisEnabled = conf.getBoolean(
OMConfigKeys.OZONE_OM_RATIS_ENABLE_KEY,
OMConfigKeys.OZONE_OM_RATIS_ENABLE_DEFAULT);
// For test purpose only
ignorePipelineinKey = conf.getBoolean(
"ozone.om.ignore.pipeline", Boolean.TRUE);
}
/**
* For subclass overriding.
*/
protected OmMetadataManagerImpl() {
OzoneConfiguration conf = new OzoneConfiguration();
this.lock = new OzoneManagerLock(conf);
this.omEpoch = 0;
perfMetrics = null;
}

public static OmMetadataManagerImpl createCheckpointMetadataManager(
Expand Down Expand Up @@ -381,8 +400,9 @@ private OmMetadataManagerImpl(OzoneConfiguration conf, File dir, String name)
lock = new OmReadOnlyLock();
omEpoch = 0;
setStore(loadDB(conf, dir, name, true,
java.util.Optional.of(Boolean.TRUE), Optional.empty()));
Optional.of(Boolean.TRUE), Optional.empty()));
initializeOmTables(CacheType.PARTIAL_CACHE, false);
perfMetrics = null;
}


Expand Down Expand Up @@ -413,13 +433,14 @@ private OmMetadataManagerImpl(OzoneConfiguration conf, File dir, String name)
checkSnapshotDirExist(checkpoint);
}
setStore(loadDB(conf, metaDir, dbName, false,
java.util.Optional.of(Boolean.TRUE),
Optional.of(Boolean.TRUE),
Optional.of(maxOpenFiles), false, false));
initializeOmTables(CacheType.PARTIAL_CACHE, false);
} catch (IOException e) {
stop();
throw e;
}
perfMetrics = null;
}

@Override
Expand Down Expand Up @@ -568,14 +589,14 @@ public void start(OzoneConfiguration configuration) throws IOException {
public static DBStore loadDB(OzoneConfiguration configuration, File metaDir)
throws IOException {
return loadDB(configuration, metaDir, OM_DB_NAME, false,
java.util.Optional.empty(), Optional.empty(), true, true);
Optional.empty(), Optional.empty(), true, true);
}

public static DBStore loadDB(OzoneConfiguration configuration, File metaDir,
String dbName, boolean readOnly,
java.util.Optional<Boolean>
Optional<Boolean>
disableAutoCompaction,
java.util.Optional<Integer> maxOpenFiles)
Optional<Integer> maxOpenFiles)
throws IOException {
return loadDB(configuration, metaDir, dbName, readOnly,
disableAutoCompaction, maxOpenFiles, true, true);
Expand All @@ -584,9 +605,9 @@ public static DBStore loadDB(OzoneConfiguration configuration, File metaDir,
@SuppressWarnings("checkstyle:parameternumber")
public static DBStore loadDB(OzoneConfiguration configuration, File metaDir,
String dbName, boolean readOnly,
java.util.Optional<Boolean>
Optional<Boolean>
disableAutoCompaction,
java.util.Optional<Integer> maxOpenFiles,
Optional<Integer> maxOpenFiles,
boolean enableCompactionDag,
boolean createCheckpointDirs)
throws IOException {
Expand Down Expand Up @@ -797,7 +818,7 @@ public DBStore getStore() {
*/
@Override
public String getVolumeKey(String volume) {
return OzoneConsts.OM_KEY_PREFIX + volume;
return OM_KEY_PREFIX + volume;
}

/**
Expand Down Expand Up @@ -1071,13 +1092,13 @@ public List<OmBucketInfo> listBuckets(final String volumeName,
List<OmBucketInfo> result = new ArrayList<>();
if (Strings.isNullOrEmpty(volumeName)) {
throw new OMException("Volume name is required.",
ResultCodes.VOLUME_NOT_FOUND);
VOLUME_NOT_FOUND);
}

String volumeNameBytes = getVolumeKey(volumeName);
if (volumeTable.get(volumeNameBytes) == null) {
throw new OMException("Volume " + volumeName + " not found.",
ResultCodes.VOLUME_NOT_FOUND);
VOLUME_NOT_FOUND);
}

String startKey;
Expand Down Expand Up @@ -1162,26 +1183,26 @@ public List<OmBucketInfo> listBuckets(final String volumeName,
public ListKeysResult listKeys(String volumeName, String bucketName,
String startKey, String keyPrefix, int maxKeys)
throws IOException {

long startNanos = Time.monotonicNowNanos();
List<OmKeyInfo> result = new ArrayList<>();
if (maxKeys <= 0) {
return new ListKeysResult(result, false);
}

if (Strings.isNullOrEmpty(volumeName)) {
throw new OMException("Volume name is required.",
ResultCodes.VOLUME_NOT_FOUND);
VOLUME_NOT_FOUND);
}

if (Strings.isNullOrEmpty(bucketName)) {
throw new OMException("Bucket name is required.",
ResultCodes.BUCKET_NOT_FOUND);
BUCKET_NOT_FOUND);
}

String bucketNameBytes = getBucketKey(volumeName, bucketName);
if (getBucketTable().get(bucketNameBytes) == null) {
throw new OMException("Bucket " + bucketName + " not found.",
ResultCodes.BUCKET_NOT_FOUND);
BUCKET_NOT_FOUND);
}

String seekKey;
Expand Down Expand Up @@ -1231,11 +1252,11 @@ public ListKeysResult listKeys(String volumeName, String bucketName,
cacheKeyMap.put(key, omKeyInfo);
}
}

long startNano, stopNano = 0;
Comment thread
tanvipenumudy marked this conversation as resolved.
Outdated
// Get maxKeys from DB if it has.

try (TableIterator<String, ? extends KeyValue<String, OmKeyInfo>>
keyIter = getKeyTable(getBucketLayout()).iterator()) {
startNano = Time.monotonicNowNanos();
KeyValue< String, OmKeyInfo > kv;
keyIter.seek(seekKey);
// we need to iterate maxKeys + 1 here because if skipStartKey is true,
Expand All @@ -1258,10 +1279,22 @@ public ListKeysResult listKeys(String volumeName, String bucketName,
break;
}
}
stopNano = Time.monotonicNowNanos();
}

boolean isTruncated = cacheKeyMap.size() > maxKeys;

if (perfMetrics != null) {
long averagePagination;
Comment thread
tanvipenumudy marked this conversation as resolved.
Outdated
if (isTruncated) {
averagePagination = maxKeys;
} else {
averagePagination = cacheKeyMap.size();
}
perfMetrics.setListKeysAveragePagination(averagePagination);
long opsPerSec =
averagePagination / (Time.monotonicNowNanos() - startNanos);
perfMetrics.setListKeysOpsPerSec(opsPerSec);
Comment thread
muskan1012 marked this conversation as resolved.
Outdated
perfMetrics.addListKeysReadFromRocksDbLatencyNs(stopNano - startNano);
}
// Finally DB entries and cache entries are merged, then return the count
// of maxKeys from the sorted map.
currentCount = 0;
Expand Down Expand Up @@ -1402,7 +1435,7 @@ public List<OmVolumeArgs> listVolumes(String userName,
int index = 0;
if (!Strings.isNullOrEmpty(startKey)) {
index = volumes.indexOf(
startKey.startsWith(OzoneConsts.OM_KEY_PREFIX) ?
startKey.startsWith(OM_KEY_PREFIX) ?
startKey.substring(1) :
startKey);

Expand All @@ -1422,7 +1455,7 @@ public List<OmVolumeArgs> listVolumes(String userName,
// this probably means om db is corrupted or some entries are
// accidentally removed.
throw new OMException("Volume info not found for " + volumeName,
ResultCodes.VOLUME_NOT_FOUND);
VOLUME_NOT_FOUND);
}
result.add(volumeArgs);
}
Expand Down Expand Up @@ -1746,7 +1779,7 @@ public ExpiredOpenKeys getExpiredOpenKeys(Duration expireThreshold,
= dbOpenKeyName.substring(lastPrefix + 1);

final OmKeyInfo info = kt.get(dbKeyName);
final boolean isHsync = java.util.Optional.ofNullable(info)
final boolean isHsync = Optional.ofNullable(info)
.map(WithMetadata::getMetadata)
.map(meta -> meta.get(OzoneConsts.HSYNC_CLIENT_ID))
.filter(id -> id.equals(clientIdString))
Expand All @@ -1762,7 +1795,7 @@ public ExpiredOpenKeys getExpiredOpenKeys(Duration expireThreshold,
.setBucketName(info.getBucketName())
.setKeyName(info.getKeyName())
.setDataSize(info.getDataSize());
java.util.Optional.ofNullable(info.getLatestVersionLocations())
Optional.ofNullable(info.getLatestVersionLocations())
.map(OmKeyLocationInfoGroup::getLocationList)
.map(Collection::stream)
.orElseGet(Stream::empty)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public void setUp() throws IOException {
conf = new OzoneConfiguration();
conf.set(OZONE_OM_DB_DIRS, folder.toAbsolutePath().toString());
conf.set(OZONE_OM_TENANT_DEV_SKIP_RANGER, "true");
omMetadataManager = new OmMetadataManagerImpl(conf, ozoneManager);
omMetadataManager = new OmMetadataManagerImpl(conf, ozoneManager, new OMPerformanceMetrics());
Comment thread
muskan1012 marked this conversation as resolved.
Outdated

createTenantInDB(TENANT_ID);
assignUserToTenantInDB(TENANT_ID, "seed-accessId1", "seed-user1", false,
Expand Down
Loading