Skip to content

Conversation

ohadzeliger
Copy link
Contributor

Add more info to the Lucene Index Operation metadata (file ID and size)
Make the LuceneGetMetadataInfo class EXPERIMENTAL (it is used from outside the RL).
Deprecate the metadata's file name list (users can substitute the new field to get access to the file names).

Resolves #3554

…ake metadata class API.EXPERIMENTAL.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@ohadzeliger ohadzeliger self-assigned this Aug 20, 2025
@ohadzeliger ohadzeliger added the enhancement New feature or request label Aug 20, 2025
@ohadzeliger ohadzeliger requested review from ScottDugas and removed request for ScottDugas August 20, 2025 15:42
@ohadzeliger ohadzeliger marked this pull request as ready for review September 4, 2025 20:59
@ohadzeliger ohadzeliger requested a review from jjezra September 4, 2025 21:09
@Nullable
private final Collection<LuceneFileInfo> detailedFileInfos;

@Deprecated(forRemoval = true)
Copy link
Contributor

Choose a reason for hiding this comment

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

There was some discussion about it, and I think that @API(API.Status.DEPRECATED) is preferred.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. I was trying to get Teamscale to not mark it as a test gap, but that didn't work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

final int fieldInfoCount,
@Nonnull final Collection<LuceneFileInfo> detailedFileInfos) {
this.documentCount = documentCount;
this.files = detailedFileInfos.stream().map(LuceneFileInfo::getName).collect(Collectors.toList());
Copy link
Contributor

Choose a reason for hiding this comment

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

Can "files" here have a null assingment, and getFiles below be something like:

if (files == null && detailedFileInfos != null) {
  return detailedFileInfos.stream().map(LuceneFileInfo::getName).collect(Collectors.toList());
} 
// else return an error...

?

I assume that since files is private, the callers will always use the getFiles function to access this data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can do that, but then we will recompute the derivation every time instead of once.

Copy link
Contributor

Choose a reason for hiding this comment

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

if (files == null && detailedFileInfos != null) {
  files = detailedFileInfos.stream().map(LuceneFileInfo::getName).collect(Collectors.toList());
  return files;
} 

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is exactly what constructors are doing - except that this way the class is immutable: First constructor has populates files and second constructor has files==null and detailedFileInfos != null and is therefore exactly what you have in your comment.
Since the files field is deprecated (to be removed once usage is gone), then we will be left with a simple constructor initialization.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the constructor to take a list and added unmodifieableList to the constructor. This allows the equals method to work properly.

@jjezra jjezra merged commit 5f28d61 into FoundationDB:main Sep 17, 2025
8 checks passed
@ohadzeliger ohadzeliger deleted the lucene-extended-metadata branch September 17, 2025 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need more info in LuceneMetadata index operation
2 participants