Skip to content

CNDB-16697 Port CNBD-16135 79f25c7cf0 to main-5.0#2242

Draft
zgorzalyj wants to merge 2 commits intomain-5.0from
CNDB-16697-main-5.0
Draft

CNDB-16697 Port CNBD-16135 79f25c7cf0 to main-5.0#2242
zgorzalyj wants to merge 2 commits intomain-5.0from
CNDB-16697-main-5.0

Conversation

@zgorzalyj
Copy link

@zgorzalyj zgorzalyj commented Feb 20, 2026

https://github.com/riptano/cndb/issues/16697

What is the issue

CNDB-16135: Descriptor uses different instances of `Path directory` for
the same table as constructor parameter

What does this PR fix and why was it fixed

Use the table directory from `Directories` in `Descriptor` as
constructor parameter. `Descriptor#directory` is still different
instance in C* due to `directory#toCanonical()` which always creates new
instance in local file system

Backport notes (main-5.0)

Cherry-picked from commit 79f25c7

Adaptations for main-5.0:

  • Added new `fromFilenameWithComponent(File tableDirectory, String name)` method
  • Adapted to use main-5.0 APIs: `formatFromName()` and `Component.parse(String, SSTableFormat)`
  • Fixed `validateAndExtractInfo()` to return `SSTableInfo` object
  • Added missing `javax.annotation.Nullable` import
  • Preserved all existing methods for backward compatibility"

jasonstack and others added 2 commits February 20, 2026 08:56
…2150)

CNDB-16135: Descriptor uses different instances of `Path directory` for
the same table as constructor parameter

Use the table directory from `Directories` in `Descriptor` as
constructor parameter . `Descriptor#directory` is still different
instance in C* due to `directory#toCanonical()` which always creates new
instance in local file system
@github-actions
Copy link

Checklist before you submit for review

  • This PR adheres to the Definition of Done
  • Make sure there is a PR in the CNDB project updating the Converged Cassandra version
  • Use NoSpamLogger for log lines that may appear frequently in the logs
  • Verify test results on Butler
  • Test coverage for new/modified code is > 80%
  • Proper code formatting
  • Proper title for each commit staring with the project-issue number, like CNDB-1234
  • Each commit has a meaningful description
  • Each commit is not very long and contains related changes
  • Renames, moves and reformatting are in distinct commits
  • All new files should contain the DataStax copyright header instead of the Apache License one

@zgorzalyj zgorzalyj requested a review from djatnieks February 20, 2026 10:02
@djatnieks
Copy link
Member

@zgorzalyj Apologies, I did not realize the specifics of this porting task - it looked small so I assumed it would be a relatively clean one. However, this an example of a CC main commit that hits areas of CC main-5.0 that have changed significantly, and not easy to understand, between these releases.

That said, I'm not sure I understand the large amount of changes to these files - I did not get the same type of results when I did:

cc-main-5.0 git:(main-5.0) ✗ git cherry-pick 79f25c7cf0
Auto-merging src/java/org/apache/cassandra/db/Directories.java
CONFLICT (content): Merge conflict in src/java/org/apache/cassandra/db/Directories.java
Auto-merging src/java/org/apache/cassandra/io/sstable/Descriptor.java
CONFLICT (content): Merge conflict in src/java/org/apache/cassandra/io/sstable/Descriptor.java
Auto-merging test/unit/org/apache/cassandra/db/DirectoriesTest.java
Auto-merging test/unit/org/apache/cassandra/io/sstable/DescriptorTest.java
CONFLICT (content): Merge conflict in test/unit/org/apache/cassandra/io/sstable/DescriptorTest.java
error: could not apply 79f25c7cf0... CNDB-16135: reuse table directory from Directories into Descriptor (#2150)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Recorded preimage for 'src/java/org/apache/cassandra/db/Directories.java'
Recorded preimage for 'src/java/org/apache/cassandra/io/sstable/Descriptor.java'
Recorded preimage for 'test/unit/org/apache/cassandra/io/sstable/DescriptorTest.java'

The scope of conflicts I got is much smaller. Perhaps it's because I usually do a simple git cherry-pick without any other options.

Even so, once I got started looking at the changes it took me more time than expected today, and I wanted to be able to have some response to give.

Most of the conflict is in Descriptor where there has been method name refactoring, e.g. fromFilenameWithComponent is now fromFileWithComponent in main-5.0. There are other changes and code sometimes was moved.

Finally, I decide to start back at the original main branch commits and understand better the original goal. My understanding is that Directories.resolve method, which appears as little-used in CC, but is actually used more from CNDB, needed a change to be able to separately pass the directory and filename to the Descriptor.fromFilenameWithComponent method.

Sometimes in these porting it's harder to make sense of the cherry-pick merge conflicts and instead switch to a more focused manual porting of code changes. In addition sometimes code from the older main branch just doesn't make sense in main-5.0 due to changes over time.

With that as a goal, I started from the changes in Directories and considered each change made in main. It didn't help that main previously only had Descriptor.fromFilenameWithComponent(File file) and the commit split this method, adding fromFilenameWithComponent(File tableDirectory, String name). While main-5.0 already has separate methods fromFileWithComponent(File file) and fromFileWithComponent(File file, boolean validateDirs).

After some trial/error, I found it was easier to make an alternate change for main-5.0 to have Directories.resolve create a new File using the desired directory and filename to call fromFileWithComponent. I think this will work - this is also an example where it makes sense to get the original author involved when it's time for review.

I pushed a branch, datastax/CNDB-16697-danj, with the result.

Take a look at it - see if it makes sense to you and if the explanations manage to make some sense. We can discuss some more next week too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants