HDDS-8655. Optimize listKeys by eliminating redundant seek.

[armstrong0704]

What changes were proposed in this pull request?

Fixes HDDS-8655 - Improve listKey performance for OBS buckets

Description

This change optimizes the listKeys operation in Ozone Manager by removing a redundant seek operation during iterator initialization. Previously, the listKeys functionality would create an iterator which implicitly sought to the beginning of the table (seekToFirst), and then immediately sought to the specific start key. This caused a double-seek, which is inefficient for RocksDB, particularly when the initial seek triggers unnecessary block loads.

Changes

• Framework: Extended the Table interface to support passing a seek key directly when creating an iterator.
• Implementation: Updated RDBTable and TypedTable to handle this new parameter.
• RocksDB: Modified RDBStoreByteArrayIterator to check for a seek key in its constructor. If one is provided, it seeks directly to that position, skipping the default behavior of seeking to the first key.
• Ozone Manager: Updated the listKeys implementation to use this new iterator approach.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-8655

How was this patch tested?

I added a new regression test, testIteratorWithSeek, to TestRDBStore to confirm that the iterator starts correctly at the designated key. I also verified thread safety with a new concurrency test, testConcurrentIteratorWithWrites, which runs a writer and a reader in parallel to ensure stability. Existing tests in TestOmMetadataManager also pass.

[ivandika3]

Thanks @armstrong0704 for the patch. Please enable the build-branch workflow in your fork.

[armstrong0704]

@ivandika3 thanks for the input. Linters are making additional formatting. Any Guidance there?

[ivandika3]

@armstrong0704 Could you disable the auto format in the linter? Afterwards, if you are using IntelliJ IDEA, you can go to "Code Style" and import the https://github.com/apache/ozone/blob/master/hadoop-hdds/dev-support/checkstyle/checkstyle.xml

[sodonnel]

Please revert all the whitespace and comment formatting changes. Its making this diff a lot bigger than it needs to be.

[armstrong0704]

@sodonnel Done.

[swamirishi]

@vyalamar do you wanna review this patch?

[adoroszlai]

Thanks @armstrong0704 for the patch.

[adoroszlai]

Suggested change

	RDBTable table, byte[] prefix, IteratorType type) {
	RDBTable table, byte[] prefix, IteratorType type) {

nit: Please disable Align when multiline in settings (if using IDEA).

[adoroszlai]

nit: Please do not format method signature like this. Whenever visibility / return type / method name / other modifiers are changed, we would have to reindent all parameters.

[adoroszlai]

Suggested change

	* @param endKey end metadata key
	* @param endKey end metadata key

[adoroszlai]

nit: let's not change this unnecessarily

Suggested change

	/**
	* Covert the given key to the {@link RAW} type.
	*/
	/** Covert the given key to the {@link RAW} type. */

[adoroszlai]

Isn't this seek now unnecessary?

[adoroszlai]

I'd like to suggest adding a helper method to verify the list of keys iterated. Something like:

  private static void assertIterator(String prefix, String seek, Table<byte[], ?> table, List<String> expected)
      throws RocksDatabaseException, CodecException {
    final byte[] prefixBytes = prefix != null ? getBytesUtf16(prefix) : null;
    final byte[] seekBytes = seek != null ? getBytesUtf16(seek) : null;
    try (TableIterator<byte[], ? extends Table.KeyValue<byte[], ?>> iter = table.iterator(prefixBytes, seekBytes)) {
      for (String e : expected) {
        assertTrue(iter.hasNext());
        assertArrayEquals(getBytesUtf16(e), iter.next().getKey());
      }
      assertFalse(iter.hasNext());
    }
  }

Then we can both simplify cases and make sure the iterator stops (like already done in two of the five).

    // Case 1: Seek to existing key, no prefix
    assertIterator(null, "a3", table, asList("a3", "a5", "b2", "b4"));

    // Case 2: Seek to non-existent key (should land on next greater), no prefix
    assertIterator(null, "a2", table, asList("a3", "a5", "b2", "b4"));

    // Case 3: Seek past all keys, no prefix
    assertIterator(null, "z9", table, emptyList());

    // Case 4: Seek with prefix
    assertIterator("b", "b3", table, singletonList("b4"));

    // Case 5: Seek with prefix to start of prefix
    assertIterator("b", "b2", table, asList("b2", "b4"));

It can be reused in testIteratorSeekEdgeCases, too.

[github-actions]

This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.

[github-actions]

This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.

[github-actions]

Thank you for your contribution. This PR is being closed due to inactivity. Please contact a maintainer if you would like to reopen it.