Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BanyanDB: Add installation check for storageOnly = true columns. #12928

Closed
wants to merge 3 commits into from

Conversation

wu-sheng
Copy link
Member

@wu-sheng wu-sheng commented Jan 4, 2025

  • If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #.
  • Update the CHANGES log.
  • BanyanDB: Add installation check for storageOnly = true columns. All columns should be @BanyanDB.MeasureField or
    in the index mode entity. Tags are for indexing columns and series IDs only.
  • Remove @BanyanDB.IndexMode from TopN implementation, as it's not necessary. Those are records, not metrics.
  • Fix, move Service, Instance, Endpoint and Process relations out of index-mode, as they are time series.

@hanahmily @wankai123 I was trying to make all storageOnly = true columns in measure as fields automatically without extra/explict annotation, but I noticed the value-column still needs that explicit annotation. So, I still keep it. WDYT?

…necessary. Those are records, not metrics.

* Fix, move `Service`, `Instance`, `Endpoint` and `Process` relations out of `index-mode`, as they are time series.
@wu-sheng wu-sheng added backend OAP backend related. enhancement Enhancement on performance or codes labels Jan 4, 2025
@wu-sheng wu-sheng added this to the 10.2.0 milestone Jan 4, 2025
Comment on lines +182 to +194
List<String> storageOnlyTags
= tagsAndFields.tags.stream()
// filter out index tags
.filter(tagMetadata -> !tagMetadata.isIndex())
// filter out seriesID tags
.filter(tagMetadata -> !seriesIDColumns.contains(tagMetadata.getTagSpec().getName()))
.map(tagMetadata -> tagMetadata.getTagSpec().getName())
.collect(Collectors.toList());
if (!storageOnlyTags.isEmpty()) {
throw new StorageException(model.getName() + ":"
+ "storage only tags(" + storageOnlyTags + ") are not recommended in measure model, "
+ "please change the entity to index mode or use field(@BanyanDB.MeasureField) instead");
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This part is the new logic. If there are storage-only tags exist in the installation stage, an exception will be thrown out.

@wu-sheng
Copy link
Member Author

wu-sheng commented Jan 5, 2025

Closing this PR, we can't do this.

In the topology query, componentIds are a part of group by, so, it has to be a tag. Meanwhile it is a filter condition, so, it doesn't need to be indexing.

@wu-sheng wu-sheng closed this Jan 5, 2025
@wu-sheng wu-sheng deleted the storage-only-field branch January 5, 2025 00:41
@wu-sheng wu-sheng restored the storage-only-field branch January 5, 2025 00:43
@wu-sheng wu-sheng deleted the storage-only-field branch January 5, 2025 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend OAP backend related. enhancement Enhancement on performance or codes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant