You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/10/24 15:09:43 UTC
[GitHub] [skywalking] lujiajing1126 opened a new pull request, #9839: Update to banyandb client 0.2
lujiajing1126 opened a new pull request, #9839:
URL: https://github.com/apache/skywalking/pull/9839
Signed-off-by: Megrez Lu <lu...@gmail.com>
- [x] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #9805 .
- [ ] Update the [`CHANGES` log](https://github.com/apache/skywalking/blob/master/docs/en/changes/changes.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] hanahmily commented on pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
hanahmily commented on PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#issuecomment-1290084616
> Besides the last one(Zipkin), we should be able to fix those TODOs at this time, right? 0.2 BanyanDB server should have provided full features for OAP already.
Besides the double field, others could be stripped out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011972437
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBIndexInstaller.java:
##########
@@ -67,13 +72,13 @@ protected boolean isExists(Model model) throws StorageException {
protected void createTable(Model model) throws StorageException {
try {
if (model.isTimeSeries() && model.isRecord()) { // stream
- Stream stream = (Stream) MetadataRegistry.INSTANCE.registerModel(model, config);
+ Stream stream = (Stream) MetadataRegistry.INSTANCE.registerModel(model, config, configService);
if (stream != null) {
log.info("install stream schema {}", model.getName());
((BanyanDBStorageClient) client).define(stream);
}
} else if (model.isTimeSeries() && !model.isRecord()) { // measure
- Measure measure = (Measure) MetadataRegistry.INSTANCE.registerModel(model, config);
+ Measure measure = (Measure) MetadataRegistry.INSTANCE.registerModel(model, config, configService);
Review Comment:
Done.
@hanahmily Shall we pre-create group for `Property`? It seems unnecessary from the current code
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1012396238
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBIndexInstaller.java:
##########
@@ -35,28 +36,32 @@
@Slf4j
public class BanyanDBIndexInstaller extends ModelInstaller {
private final BanyanDBStorageConfig config;
+ private final ConfigService configService;
public BanyanDBIndexInstaller(Client client, ModuleManager moduleManager, BanyanDBStorageConfig config) {
super(client, moduleManager);
this.config = config;
+ this.configService = moduleManager.find(CoreModule.NAME).provider().getService(ConfigService.class);
}
@Override
protected boolean isExists(Model model) throws StorageException {
- final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config);
+ final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config, configService);
try {
final BanyanDBClient c = ((BanyanDBStorageClient) this.client).client;
- // first check group
- Group g = metadata.getOrCreateGroup(c);
- if (g == null) {
- throw new StorageException("fail to create group " + metadata.getGroup());
+ // first check resource existence and create group if necessary
+ final boolean resourceExist = metadata.checkResourceExistence(c);
+ if (!resourceExist) {
+ return false;
}
- log.info("group {} created", g.name());
+
// then check entity schema
if (metadata.findRemoteSchema(c).isPresent()) {
- MetadataRegistry.INSTANCE.registerModel(model, config);
+ MetadataRegistry.INSTANCE.registerModel(model, config, configService);
Review Comment:
This line cannot be removed. Since even if schema is already registered in the BanyanDB, we still have to register it locally for query and write.
So these lines are restored.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#issuecomment-1289207702
<img width="1079" alt="image" src="https://user-images.githubusercontent.com/5441976/197563928-dff31fbc-f208-4c63-8f94-e74f94f446aa.png">
Besides the last one(Zipkin), we should be able to fix those TODOs at this time, right? 0.2 BanyanDB server should have provided full features for OAP already.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] hanahmily commented on pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
hanahmily commented on PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#issuecomment-1296437545
@kezhenxu94 skywalking-eye fails to handle some repos' license:
```
ERROR failed to identify the licenses of following packages (2):
build.buf.protoc-gen-validate:pgv-java-stub
build.buf.protoc-gen-validate:protoc-gen-validate
Error: Process completed with exit code 1.
```
https://github.com/apache/skywalking/actions/runs/3342606435/jobs/5537555471
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1009128652
##########
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/storage/annotation/BanyanDB.java:
##########
@@ -91,4 +91,19 @@
@interface NoIndexing {
}
+
+ /**
+ * Additional information for constructing Index in BanyanDB.
+ *
+ * @since 9.3.0
+ */
+ @Target({ElementType.FIELD})
+ @Retention(RetentionPolicy.RUNTIME)
+ @interface IndexRule {
+ IndexType indexType() default IndexType.INVERTED;
+
+ enum IndexType {
+ INVERTED, TREE;
Review Comment:
Please provide a simple explanation for those two index types.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1009130462
##########
docs/en/changes/changes.md:
##########
@@ -3,6 +3,7 @@
#### Project
* Bump up the embedded `swctl` version in OAP Docker image.
+* Migrate to BanyanDB v0.2.0.
Review Comment:
Let's add these key changes into changelog as well
<img width="527" alt="image" src="https://user-images.githubusercontent.com/5441976/198961517-93b521be-c53e-4cd0-9198-d299350551ba.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1012397912
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBIndexInstaller.java:
##########
@@ -35,28 +36,32 @@
@Slf4j
public class BanyanDBIndexInstaller extends ModelInstaller {
private final BanyanDBStorageConfig config;
+ private final ConfigService configService;
public BanyanDBIndexInstaller(Client client, ModuleManager moduleManager, BanyanDBStorageConfig config) {
super(client, moduleManager);
this.config = config;
+ this.configService = moduleManager.find(CoreModule.NAME).provider().getService(ConfigService.class);
}
@Override
protected boolean isExists(Model model) throws StorageException {
- final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config);
+ final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config, configService);
try {
final BanyanDBClient c = ((BanyanDBStorageClient) this.client).client;
- // first check group
- Group g = metadata.getOrCreateGroup(c);
- if (g == null) {
- throw new StorageException("fail to create group " + metadata.getGroup());
+ // first check resource existence and create group if necessary
+ final boolean resourceExist = metadata.checkResourceExistence(c);
+ if (!resourceExist) {
+ return false;
}
- log.info("group {} created", g.name());
+
// then check entity schema
if (metadata.findRemoteSchema(c).isPresent()) {
- MetadataRegistry.INSTANCE.registerModel(model, config);
+ MetadataRegistry.INSTANCE.registerModel(model, config, configService);
Review Comment:
Do we have a better to make codes more clear? Such as, when we do `isPresent`, could we use the metadata of existing measure/stream to build this local registration automatically?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#issuecomment-1301711376
FYI @wankai123 This would help a lot to finish BanyanDB storage for Zipkin part(for query)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1012418903
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBIndexInstaller.java:
##########
@@ -35,28 +36,32 @@
@Slf4j
public class BanyanDBIndexInstaller extends ModelInstaller {
private final BanyanDBStorageConfig config;
+ private final ConfigService configService;
public BanyanDBIndexInstaller(Client client, ModuleManager moduleManager, BanyanDBStorageConfig config) {
super(client, moduleManager);
this.config = config;
+ this.configService = moduleManager.find(CoreModule.NAME).provider().getService(ConfigService.class);
}
@Override
protected boolean isExists(Model model) throws StorageException {
- final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config);
+ final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config, configService);
try {
final BanyanDBClient c = ((BanyanDBStorageClient) this.client).client;
- // first check group
- Group g = metadata.getOrCreateGroup(c);
- if (g == null) {
- throw new StorageException("fail to create group " + metadata.getGroup());
+ // first check resource existence and create group if necessary
+ final boolean resourceExist = metadata.checkResourceExistence(c);
+ if (!resourceExist) {
+ return false;
}
- log.info("group {} created", g.name());
+
// then check entity schema
if (metadata.findRemoteSchema(c).isPresent()) {
- MetadataRegistry.INSTANCE.registerModel(model, config);
+ MetadataRegistry.INSTANCE.registerModel(model, config, configService);
Review Comment:
> Do we have a better to make codes more clear? Such as, when we do `isPresent`, could we use the metadata of existing measure/stream to build this local registration automatically?
I don't think it is enough to extract all necessary metadata with the schema in the BanyanDB. For example,
```java
public static class StreamToStorage implements Convert2Storage<StreamWrite> {
...
@Override
public void accept(String fieldName, Object fieldValue) {
MetadataRegistry.ColumnSpec columnSpec = this.schema.getSpec(fieldName);
if (columnSpec == null) {
throw new IllegalArgumentException("fail to find field[" + fieldName + "]");
}
try {
this.streamWrite.tag(fieldName, buildTag(fieldValue, columnSpec.getColumnClass()));
} catch (BanyanDBException ex) {
log.error("fail to add tag", ex);
}
}
}
private static Serializable<BanyandbModel.TagValue> buildTag(Object value, final Class<?> clazz) {
if (int.class.equals(clazz) || Integer.class.equals(clazz)) {
return TagAndValue.longTagValue(((Number) value).longValue());
} else if (Long.class.equals(clazz) || long.class.equals(clazz)) {
return TagAndValue.longTagValue((Long) value);
} else if (String.class.equals(clazz)) {
return TagAndValue.stringTagValue((String) value);
} else if (Double.class.equals(clazz) || double.class.equals(clazz)) {
return TagAndValue.binaryTagValue(ByteUtil.double2Bytes((double) value));
} else if (StorageDataComplexObject.class.isAssignableFrom(clazz)) {
return TagAndValue.stringTagValue(((StorageDataComplexObject<?>) value).toStorageData());
} else if (clazz.isEnum()) {
return TagAndValue.longTagValue((int) value);
} else if (JsonObject.class.equals(clazz)) {
return TagAndValue.stringTagValue((String) value);
} else if (byte[].class.equals(clazz)) {
return TagAndValue.stringTagValue((String) value);
}
throw new IllegalStateException(clazz.getSimpleName() + " is not supported");
}
```
The mapping between data types in BanyanDB and OAP is not a surjection, i.e. one-to-one mapping.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#issuecomment-1294799778
> Is this ready? We only leave double and new top N to add in next version? If so, could you create an issue for 9.4.0 to track this?
Yes. It is generally ready for review. I will fix CI and E2E.
Created #9864
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011164813
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/MetadataRegistry.java:
##########
@@ -293,21 +335,41 @@ public SchemaMetadata parseMetadata(Model model, BanyanDBStorageConfig config) {
@Data
public static class SchemaMetadata {
private final String group;
- private final String name;
+ /**
+ * name of the {@link Model}
+ */
+ private final String modelName;
private final Kind kind;
-
+ /**
+ * down-sampling of the {@link Model}
+ */
+ private final DownSampling downSampling;
private final int shard;
private final int blockIntervalHrs;
private final int segmentIntervalHrs;
private final int ttlDays;
+ /**
+ * Format the entity name for BanyanDB
+ *
+ * @param modelName name of the model
+ * @param downSampling not used if it is {@link DownSampling#None}
+ * @return entity (e.g. measure, stream) name
+ */
+ static String formatName(String modelName, DownSampling downSampling) {
+ if (downSampling == null || downSampling == DownSampling.None) {
+ return modelName;
+ }
+ return modelName + ":" + downSampling.getName();
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011299221
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBConverter.java:
##########
@@ -215,11 +218,19 @@ public static class StorageToMeasure implements Convert2Entity {
private final MetadataRegistry.Schema schema;
private final DataPoint dataPoint;
- public StorageToMeasure(String modelName, DataPoint dataPoint) {
- this.schema = MetadataRegistry.INSTANCE.findMetadata(modelName);
+ public StorageToMeasure(MetadataRegistry.Schema schema, DataPoint dataPoint) {
+ this.schema = schema;
this.dataPoint = dataPoint;
}
+ /**
+ * Currently, only used by {@link org.apache.skywalking.oap.server.storage.plugin.banyandb.measure.BanyanDBNetworkAddressAliasDAO}.
+ * The default DownSampling strategy, i.e. {@link DownSampling#Minute} is assumed in this case.
+ */
+ public StorageToMeasure(String measureModelName, DataPoint dataPoint) {
+ this(checkNotNull(MetadataRegistry.INSTANCE.findMetadata(measureModelName, DownSampling.Minute), "measure schema"), dataPoint);
Review Comment:
I read the comments, why don't we move this special case into the BanyanDBNetworkAddressAliasDAO? This seems a thing not belonging to the converter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011771605
##########
oap-server/server-starter/src/main/resources/application.yml:
##########
@@ -236,6 +236,12 @@ storage:
superDatasetShardsFactor: ${SW_STORAGE_BANYANDB_SUPERDATASET_SHARDS_FACTOR:2}
concurrentWriteThreads: ${SW_STORAGE_BANYANDB_CONCURRENT_WRITE_THREADS:15}
profileTaskQueryMaxSize: ${SW_STORAGE_BANYANDB_PROFILE_TASK_QUERY_MAX_SIZE:200} # the max number of fetch task in a request
+ streamBlockInterval: ${SW_STORAGE_BANYANDB_STREAM_BLOCK_INTERVAL:4} # Unit is Hours
+ streamSegmentInterval: ${SW_STORAGE_BANYANDB_STREAM_SEGMENT_INTERVAL:24} # Unit is Hours
+ streamTTL: ${SW_STORAGE_BANYANDB_STREAM_TTL:24} # Unit is Days
Review Comment:
Fixed
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/measure/BanyanDBMetadataQueryDAO.java:
##########
@@ -230,18 +230,18 @@ public List<Process> listProcesses(String serviceInstanceId, Duration duration,
long lastPingStartTimeBucket = duration.getStartTimeBucket();
long lastPingEndTimeBucket = duration.getEndTimeBucket();
Review Comment:
Removed
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/stream/BanyanDBProfileThreadSnapshotQueryDAO.java:
##########
@@ -105,43 +103,40 @@ public void apply(StreamQuery query) {
segmentIds.add(rowEntity.getTagValue(ProfileThreadSnapshotRecord.SEGMENT_ID));
}
- // TODO: support `IN` or `OR` logic operation in BanyanDB
- List<BasicTrace> basicTraces = new ArrayList<>();
- for (String segmentID : segmentIds) {
- final StreamQueryResponse segmentRecordResp = query(SegmentRecord.INDEX_NAME,
- TAGS_TRACE,
- new QueryBuilder<StreamQuery>() {
- @Override
- public void apply(StreamQuery traceQuery) {
- traceQuery.and(eq(SegmentRecord.SEGMENT_ID, segmentID));
- }
- });
-
- for (final RowEntity row : segmentRecordResp.getElements()) {
- BasicTrace basicTrace = new BasicTrace();
-
- basicTrace.setSegmentId(row.getId());
- basicTrace.setStart(String.valueOf((Number) row.getTagValue(SegmentRecord.START_TIME)));
- basicTrace.getEndpointNames().add(IDManager.EndpointID.analysisId(
- row.getTagValue(SegmentRecord.ENDPOINT_ID)
- ).getEndpointName());
- basicTrace.setDuration(((Number) row.getTagValue(SegmentRecord.LATENCY)).intValue());
- basicTrace.setError(BooleanUtils.valueToBoolean(
- ((Number) row.getTagValue(SegmentRecord.IS_ERROR)).intValue()
- ));
- basicTrace.getTraceIds().add(row.getTagValue(SegmentRecord.TRACE_ID));
-
- basicTraces.add(basicTrace);
- }
+ if (segmentIds.isEmpty()) {
+ return Collections.emptyList();
}
- // TODO: Sort in DB with DESC
- basicTraces = basicTraces.stream()
- // comparing start_time
- .sorted(Comparator.comparing((Function<BasicTrace, Long>) basicTrace -> Long.parseLong(basicTrace.getStart()))
- // and sort in reverse order
- .reversed())
- .collect(Collectors.toList());
+ final StreamQueryResponse segmentRecordResp = query(SegmentRecord.INDEX_NAME,
+ TAGS_TRACE,
+ new QueryBuilder<StreamQuery>() {
+ @Override
+ public void apply(StreamQuery traceQuery) {
+ for (final String segmentID : segmentIds) {
+ traceQuery.or(eq(SegmentRecord.SEGMENT_ID, segmentID));
+ traceQuery.setLimit(segmentIds.size());
+ traceQuery.setOrderBy(desc(SegmentRecord.START_TIME));
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011773618
##########
docs/en/changes/changes.md:
##########
@@ -93,7 +105,7 @@
to change the OAP backend service addresses, like `SW_OAP_ADDRESS=localhost:12800,localhost:12801`, and use
environment
variable `SW_SERVER_PORT` to change the port. Other Spring-related configurations don't take effect anymore.
-* Polish the endpoint list graph.
+* Polish the endpoint list graph.g
Review Comment:
Oh, deleted
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] sonatype-lift[bot] commented on pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#issuecomment-1289233081
:warning: **6 God Classes** were detected by Lift in this project. [Visit the Lift web console](https://lift.sonatype.com/results/github.com/apache/skywalking/01GG58NPVW7V90K029S27CHM66?tab=technical-debt&utm_source=github.com&utm_campaign=lift-comment&utm_content=apache\%20skywalking) for more details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011827260
##########
oap-server/server-starter/src/main/resources/application.yml:
##########
@@ -236,6 +236,10 @@ storage:
superDatasetShardsFactor: ${SW_STORAGE_BANYANDB_SUPERDATASET_SHARDS_FACTOR:2}
concurrentWriteThreads: ${SW_STORAGE_BANYANDB_CONCURRENT_WRITE_THREADS:15}
profileTaskQueryMaxSize: ${SW_STORAGE_BANYANDB_PROFILE_TASK_QUERY_MAX_SIZE:200} # the max number of fetch task in a request
+ streamBlockInterval: ${SW_STORAGE_BANYANDB_STREAM_BLOCK_INTERVAL:4} # Unit is Hours
+ streamSegmentInterval: ${SW_STORAGE_BANYANDB_STREAM_SEGMENT_INTERVAL:24} # Unit is Hours
+ measureBlockInterval: ${SW_STORAGE_BANYANDB_MEASURE_BLOCK_INTERVAL:4} # Unit is Hours
+ measureSegmentInterval: ${SW_STORAGE_BANYANDB_MEASURE_SEGMENT_INTERVAL:24} # Unit is Hours
Review Comment:
> backend-storage.md should be updated as well.
There is a TODO doc fix
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011969967
##########
oap-server/server-starter/src/main/resources/application.yml:
##########
@@ -236,6 +236,10 @@ storage:
superDatasetShardsFactor: ${SW_STORAGE_BANYANDB_SUPERDATASET_SHARDS_FACTOR:2}
concurrentWriteThreads: ${SW_STORAGE_BANYANDB_CONCURRENT_WRITE_THREADS:15}
profileTaskQueryMaxSize: ${SW_STORAGE_BANYANDB_PROFILE_TASK_QUERY_MAX_SIZE:200} # the max number of fetch task in a request
+ streamBlockInterval: ${SW_STORAGE_BANYANDB_STREAM_BLOCK_INTERVAL:4} # Unit is Hours
+ streamSegmentInterval: ${SW_STORAGE_BANYANDB_STREAM_SEGMENT_INTERVAL:24} # Unit is Hours
+ measureBlockInterval: ${SW_STORAGE_BANYANDB_MEASURE_BLOCK_INTERVAL:4} # Unit is Hours
+ measureSegmentInterval: ${SW_STORAGE_BANYANDB_MEASURE_SEGMENT_INTERVAL:24} # Unit is Hours
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011974585
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBIndexInstaller.java:
##########
@@ -67,13 +72,13 @@ protected boolean isExists(Model model) throws StorageException {
protected void createTable(Model model) throws StorageException {
try {
if (model.isTimeSeries() && model.isRecord()) { // stream
- Stream stream = (Stream) MetadataRegistry.INSTANCE.registerModel(model, config);
+ Stream stream = (Stream) MetadataRegistry.INSTANCE.registerModel(model, config, configService);
if (stream != null) {
log.info("install stream schema {}", model.getName());
((BanyanDBStorageClient) client).define(stream);
}
} else if (model.isTimeSeries() && !model.isRecord()) { // measure
- Measure measure = (Measure) MetadataRegistry.INSTANCE.registerModel(model, config);
+ Measure measure = (Measure) MetadataRegistry.INSTANCE.registerModel(model, config, configService);
Review Comment:
If it isn't required, please add comments here to make this clear.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] hanahmily commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
hanahmily commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011245540
##########
docs/en/changes/changes.md:
##########
@@ -93,7 +105,7 @@
to change the OAP backend service addresses, like `SW_OAP_ADDRESS=localhost:12800,localhost:12801`, and use
environment
variable `SW_SERVER_PORT` to change the port. Other Spring-related configurations don't take effect anymore.
-* Polish the endpoint list graph.
+* Polish the endpoint list graph.g
Review Comment:
Typo?
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBConverter.java:
##########
@@ -215,11 +218,19 @@ public static class StorageToMeasure implements Convert2Entity {
private final MetadataRegistry.Schema schema;
private final DataPoint dataPoint;
- public StorageToMeasure(String modelName, DataPoint dataPoint) {
- this.schema = MetadataRegistry.INSTANCE.findMetadata(modelName);
+ public StorageToMeasure(MetadataRegistry.Schema schema, DataPoint dataPoint) {
+ this.schema = schema;
this.dataPoint = dataPoint;
}
+ /**
+ * Currently, only used by {@link org.apache.skywalking.oap.server.storage.plugin.banyandb.measure.BanyanDBNetworkAddressAliasDAO}.
+ * The default DownSampling strategy, i.e. {@link DownSampling#Minute} is assumed in this case.
+ */
+ public StorageToMeasure(String measureModelName, DataPoint dataPoint) {
+ this(checkNotNull(MetadataRegistry.INSTANCE.findMetadata(measureModelName, DownSampling.Minute), "measure schema"), dataPoint);
Review Comment:
@wu-sheng Could we make such assumption?
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/stream/BanyanDBProfileThreadSnapshotQueryDAO.java:
##########
@@ -105,43 +103,40 @@ public void apply(StreamQuery query) {
segmentIds.add(rowEntity.getTagValue(ProfileThreadSnapshotRecord.SEGMENT_ID));
}
- // TODO: support `IN` or `OR` logic operation in BanyanDB
- List<BasicTrace> basicTraces = new ArrayList<>();
- for (String segmentID : segmentIds) {
- final StreamQueryResponse segmentRecordResp = query(SegmentRecord.INDEX_NAME,
- TAGS_TRACE,
- new QueryBuilder<StreamQuery>() {
- @Override
- public void apply(StreamQuery traceQuery) {
- traceQuery.and(eq(SegmentRecord.SEGMENT_ID, segmentID));
- }
- });
-
- for (final RowEntity row : segmentRecordResp.getElements()) {
- BasicTrace basicTrace = new BasicTrace();
-
- basicTrace.setSegmentId(row.getId());
- basicTrace.setStart(String.valueOf((Number) row.getTagValue(SegmentRecord.START_TIME)));
- basicTrace.getEndpointNames().add(IDManager.EndpointID.analysisId(
- row.getTagValue(SegmentRecord.ENDPOINT_ID)
- ).getEndpointName());
- basicTrace.setDuration(((Number) row.getTagValue(SegmentRecord.LATENCY)).intValue());
- basicTrace.setError(BooleanUtils.valueToBoolean(
- ((Number) row.getTagValue(SegmentRecord.IS_ERROR)).intValue()
- ));
- basicTrace.getTraceIds().add(row.getTagValue(SegmentRecord.TRACE_ID));
-
- basicTraces.add(basicTrace);
- }
+ if (segmentIds.isEmpty()) {
+ return Collections.emptyList();
}
- // TODO: Sort in DB with DESC
- basicTraces = basicTraces.stream()
- // comparing start_time
- .sorted(Comparator.comparing((Function<BasicTrace, Long>) basicTrace -> Long.parseLong(basicTrace.getStart()))
- // and sort in reverse order
- .reversed())
- .collect(Collectors.toList());
+ final StreamQueryResponse segmentRecordResp = query(SegmentRecord.INDEX_NAME,
+ TAGS_TRACE,
+ new QueryBuilder<StreamQuery>() {
+ @Override
+ public void apply(StreamQuery traceQuery) {
+ for (final String segmentID : segmentIds) {
+ traceQuery.or(eq(SegmentRecord.SEGMENT_ID, segmentID));
+ traceQuery.setLimit(segmentIds.size());
+ traceQuery.setOrderBy(desc(SegmentRecord.START_TIME));
Review Comment:
They should be out of the loop.
And add a "todo" to use "in" in place of "or" later.
##########
oap-server/server-starter/src/main/resources/application.yml:
##########
@@ -236,6 +236,12 @@ storage:
superDatasetShardsFactor: ${SW_STORAGE_BANYANDB_SUPERDATASET_SHARDS_FACTOR:2}
concurrentWriteThreads: ${SW_STORAGE_BANYANDB_CONCURRENT_WRITE_THREADS:15}
profileTaskQueryMaxSize: ${SW_STORAGE_BANYANDB_PROFILE_TASK_QUERY_MAX_SIZE:200} # the max number of fetch task in a request
+ streamBlockInterval: ${SW_STORAGE_BANYANDB_STREAM_BLOCK_INTERVAL:4} # Unit is Hours
+ streamSegmentInterval: ${SW_STORAGE_BANYANDB_STREAM_SEGMENT_INTERVAL:24} # Unit is Hours
+ streamTTL: ${SW_STORAGE_BANYANDB_STREAM_TTL:24} # Unit is Days
Review Comment:
Could we read TTLs from the core's `recordTTL` and `metricTTL`? `segmentInterval` should not be greater than them.
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/measure/BanyanDBMetadataQueryDAO.java:
##########
@@ -230,18 +230,18 @@ public List<Process> listProcesses(String serviceInstanceId, Duration duration,
long lastPingStartTimeBucket = duration.getStartTimeBucket();
long lastPingEndTimeBucket = duration.getEndTimeBucket();
Review Comment:
redandunt variable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1009245251
##########
.licenserc.yaml:
##########
@@ -119,3 +119,9 @@ dependency:
- name: com.google.flatbuffers:flatbuffers-java
version: 1.12.0
license: Apache-2.0
+ - name: build.buf.protoc-gen-validate:pgv-java-stub
+ version: 0.6.13
+ license: Apache-2.0
+ - name: build.buf.protoc-gen-validate:protoc-gen-validate
+ version: 0.6.13
+ license: Apache-2.0
Review Comment:
LICENSE regenerated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#issuecomment-1294526887
Is this ready? We only leave double and new top N to add in next version? If so, could you create an issue for 9.4.0 to track this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1003450439
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/MetadataRegistry.java:
##########
@@ -322,9 +323,9 @@ public Group getOrCreateGroup(BanyanDBClient client) throws BanyanDBException {
}
switch (kind) {
case STREAM:
- return client.define(Group.create(this.group, Catalog.STREAM, this.shard, 0, Duration.ofDays(7)));
+ return client.define(Group.create(this.group, Catalog.STREAM, this.shard, IntervalRule.create(IntervalRule.Unit.HOUR, 4), IntervalRule.create(IntervalRule.Unit.DAY, 1), IntervalRule.create(IntervalRule.Unit.DAY, 7)));
case MEASURE:
- return client.define(Group.create(this.group, Catalog.MEASURE, this.shard, 12, Duration.ofDays(7)));
+ return client.define(Group.create(this.group, Catalog.MEASURE, this.shard, IntervalRule.create(IntervalRule.Unit.HOUR, 4), IntervalRule.create(IntervalRule.Unit.HOUR, 24), IntervalRule.create(IntervalRule.Unit.DAY, 7)));
Review Comment:
I think we should provide these as storage settings. These are polices controlling segment and blocks creating, right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] hanahmily merged pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
hanahmily merged PR #9839:
URL: https://github.com/apache/skywalking/pull/9839
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011821819
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBIndexInstaller.java:
##########
@@ -35,28 +36,32 @@
@Slf4j
public class BanyanDBIndexInstaller extends ModelInstaller {
private final BanyanDBStorageConfig config;
+ private final ConfigService configService;
public BanyanDBIndexInstaller(Client client, ModuleManager moduleManager, BanyanDBStorageConfig config) {
super(client, moduleManager);
this.config = config;
+ this.configService = moduleManager.find(CoreModule.NAME).provider().getService(ConfigService.class);
}
@Override
protected boolean isExists(Model model) throws StorageException {
- final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config);
+ final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config, configService);
try {
final BanyanDBClient c = ((BanyanDBStorageClient) this.client).client;
- // first check group
- Group g = metadata.getOrCreateGroup(c);
- if (g == null) {
- throw new StorageException("fail to create group " + metadata.getGroup());
+ // first check resource existence and create group if necessary
+ final boolean resourceExist = metadata.checkResourceExistence(c);
+ if (!resourceExist) {
+ return false;
}
- log.info("group {} created", g.name());
+
// then check entity schema
if (metadata.findRemoteSchema(c).isPresent()) {
- MetadataRegistry.INSTANCE.registerModel(model, config);
+ MetadataRegistry.INSTANCE.registerModel(model, config, configService);
Review Comment:
Also, you have this
```
// normally, we should not reach here
return false;
```
If it can't be reached, we should face an IllegalStateException
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1007577000
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/MetadataRegistry.java:
##########
@@ -322,9 +323,9 @@ public Group getOrCreateGroup(BanyanDBClient client) throws BanyanDBException {
}
switch (kind) {
case STREAM:
- return client.define(Group.create(this.group, Catalog.STREAM, this.shard, 0, Duration.ofDays(7)));
+ return client.define(Group.create(this.group, Catalog.STREAM, this.shard, IntervalRule.create(IntervalRule.Unit.HOUR, 4), IntervalRule.create(IntervalRule.Unit.DAY, 1), IntervalRule.create(IntervalRule.Unit.DAY, 7)));
case MEASURE:
- return client.define(Group.create(this.group, Catalog.MEASURE, this.shard, 12, Duration.ofDays(7)));
+ return client.define(Group.create(this.group, Catalog.MEASURE, this.shard, IntervalRule.create(IntervalRule.Unit.HOUR, 4), IntervalRule.create(IntervalRule.Unit.HOUR, 24), IntervalRule.create(IntervalRule.Unit.DAY, 7)));
Review Comment:
Added
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1012420685
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBIndexInstaller.java:
##########
@@ -35,28 +36,32 @@
@Slf4j
public class BanyanDBIndexInstaller extends ModelInstaller {
private final BanyanDBStorageConfig config;
+ private final ConfigService configService;
public BanyanDBIndexInstaller(Client client, ModuleManager moduleManager, BanyanDBStorageConfig config) {
super(client, moduleManager);
this.config = config;
+ this.configService = moduleManager.find(CoreModule.NAME).provider().getService(ConfigService.class);
}
@Override
protected boolean isExists(Model model) throws StorageException {
- final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config);
+ final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config, configService);
try {
final BanyanDBClient c = ((BanyanDBStorageClient) this.client).client;
- // first check group
- Group g = metadata.getOrCreateGroup(c);
- if (g == null) {
- throw new StorageException("fail to create group " + metadata.getGroup());
+ // first check resource existence and create group if necessary
+ final boolean resourceExist = metadata.checkResourceExistence(c);
+ if (!resourceExist) {
+ return false;
}
- log.info("group {} created", g.name());
+
// then check entity schema
if (metadata.findRemoteSchema(c).isPresent()) {
- MetadataRegistry.INSTANCE.registerModel(model, config);
+ MetadataRegistry.INSTANCE.registerModel(model, config, configService);
Review Comment:
> I see, then how about we separating local registration logic as new methods? Remote registration could call local registration automatically for sure.
`registerMeasureModel` and `registerStreamModel` are only responsible for registering local cache.
The `define` method is for remote registration.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1009127296
##########
.licenserc.yaml:
##########
@@ -119,3 +119,9 @@ dependency:
- name: com.google.flatbuffers:flatbuffers-java
version: 1.12.0
license: Apache-2.0
+ - name: build.buf.protoc-gen-validate:pgv-java-stub
+ version: 0.6.13
+ license: Apache-2.0
+ - name: build.buf.protoc-gen-validate:protoc-gen-validate
+ version: 0.6.13
+ license: Apache-2.0
Review Comment:
As these are added, you should update license files accordingly.
##########
docs/en/changes/changes.md:
##########
@@ -3,6 +3,7 @@
#### Project
* Bump up the embedded `swctl` version in OAP Docker image.
+* Migrate to BanyanDB v0.2.0.
Review Comment:
I think this should be in `backend` only?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1009128040
##########
docs/en/changes/changes.md:
##########
@@ -3,6 +3,7 @@
#### Project
* Bump up the embedded `swctl` version in OAP Docker image.
+* Migrate to BanyanDB v0.2.0.
Review Comment:
Also, please make this a breaking change. BanyanDB server breaks the protocol, and we change the ways to store data.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] hanahmily commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
hanahmily commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1012431914
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBIndexInstaller.java:
##########
@@ -67,13 +72,13 @@ protected boolean isExists(Model model) throws StorageException {
protected void createTable(Model model) throws StorageException {
try {
if (model.isTimeSeries() && model.isRecord()) { // stream
- Stream stream = (Stream) MetadataRegistry.INSTANCE.registerModel(model, config);
+ Stream stream = (Stream) MetadataRegistry.INSTANCE.registerModel(model, config, configService);
if (stream != null) {
log.info("install stream schema {}", model.getName());
((BanyanDBStorageClient) client).define(stream);
}
} else if (model.isTimeSeries() && !model.isRecord()) { // measure
- Measure measure = (Measure) MetadataRegistry.INSTANCE.registerModel(model, config);
+ Measure measure = (Measure) MetadataRegistry.INSTANCE.registerModel(model, config, configService);
Review Comment:
We will create a validation to check it as https://github.com/apache/skywalking/issues/9902. Please add comments here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1003445257
##########
oap-server-bom/pom.xml:
##########
@@ -73,7 +73,7 @@
<awaitility.version>3.0.0</awaitility.version>
<httpcore.version>4.4.13</httpcore.version>
<commons-compress.version>1.21</commons-compress.version>
- <banyandb-java-client.version>0.1.0</banyandb-java-client.version>
+ <banyandb-java-client.version>0.2.0-SNAPSHOT</banyandb-java-client.version>
Review Comment:
You need a repository to get this version, I think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1009244993
##########
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/storage/annotation/BanyanDB.java:
##########
@@ -91,4 +91,19 @@
@interface NoIndexing {
}
+
+ /**
+ * Additional information for constructing Index in BanyanDB.
+ *
+ * @since 9.3.0
+ */
+ @Target({ElementType.FIELD})
+ @Retention(RetentionPolicy.RUNTIME)
+ @interface IndexRule {
+ IndexType indexType() default IndexType.INVERTED;
+
+ enum IndexType {
+ INVERTED, TREE;
Review Comment:
Added
##########
docs/en/changes/changes.md:
##########
@@ -3,6 +3,7 @@
#### Project
* Bump up the embedded `swctl` version in OAP Docker image.
+* Migrate to BanyanDB v0.2.0.
Review Comment:
Added
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1012420001
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBIndexInstaller.java:
##########
@@ -35,28 +36,32 @@
@Slf4j
public class BanyanDBIndexInstaller extends ModelInstaller {
private final BanyanDBStorageConfig config;
+ private final ConfigService configService;
public BanyanDBIndexInstaller(Client client, ModuleManager moduleManager, BanyanDBStorageConfig config) {
super(client, moduleManager);
this.config = config;
+ this.configService = moduleManager.find(CoreModule.NAME).provider().getService(ConfigService.class);
}
@Override
protected boolean isExists(Model model) throws StorageException {
- final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config);
+ final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config, configService);
try {
final BanyanDBClient c = ((BanyanDBStorageClient) this.client).client;
- // first check group
- Group g = metadata.getOrCreateGroup(c);
- if (g == null) {
- throw new StorageException("fail to create group " + metadata.getGroup());
+ // first check resource existence and create group if necessary
+ final boolean resourceExist = metadata.checkResourceExistence(c);
+ if (!resourceExist) {
+ return false;
}
- log.info("group {} created", g.name());
+
// then check entity schema
if (metadata.findRemoteSchema(c).isPresent()) {
- MetadataRegistry.INSTANCE.registerModel(model, config);
+ MetadataRegistry.INSTANCE.registerModel(model, config, configService);
Review Comment:
If we want to do so, I would suggest we add more overloading functions to the `Convert2Storage` interface. For example,
```java
public interface Convert2Storage<R> {
void accept(String fieldName, String fieldValue);
void accept(String fieldName, int fieldValue);
void accept(String fieldName, long fieldValue);
void accept(String fieldName, double fieldValue);
// etc.
}
```
One for each column type in OAP, so the type can be determined at compile time.
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBIndexInstaller.java:
##########
@@ -35,28 +36,32 @@
@Slf4j
public class BanyanDBIndexInstaller extends ModelInstaller {
private final BanyanDBStorageConfig config;
+ private final ConfigService configService;
public BanyanDBIndexInstaller(Client client, ModuleManager moduleManager, BanyanDBStorageConfig config) {
super(client, moduleManager);
this.config = config;
+ this.configService = moduleManager.find(CoreModule.NAME).provider().getService(ConfigService.class);
}
@Override
protected boolean isExists(Model model) throws StorageException {
- final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config);
+ final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config, configService);
try {
final BanyanDBClient c = ((BanyanDBStorageClient) this.client).client;
- // first check group
- Group g = metadata.getOrCreateGroup(c);
- if (g == null) {
- throw new StorageException("fail to create group " + metadata.getGroup());
+ // first check resource existence and create group if necessary
+ final boolean resourceExist = metadata.checkResourceExistence(c);
+ if (!resourceExist) {
+ return false;
}
- log.info("group {} created", g.name());
+
// then check entity schema
if (metadata.findRemoteSchema(c).isPresent()) {
- MetadataRegistry.INSTANCE.registerModel(model, config);
+ MetadataRegistry.INSTANCE.registerModel(model, config, configService);
Review Comment:
If we want to do so, I would suggest adding more overloading functions to the `Convert2Storage` interface. For example,
```java
public interface Convert2Storage<R> {
void accept(String fieldName, String fieldValue);
void accept(String fieldName, int fieldValue);
void accept(String fieldName, long fieldValue);
void accept(String fieldName, double fieldValue);
// etc.
}
```
One for each column type in OAP, so the type can be determined at compile time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011970772
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBIndexInstaller.java:
##########
@@ -35,28 +36,32 @@
@Slf4j
public class BanyanDBIndexInstaller extends ModelInstaller {
private final BanyanDBStorageConfig config;
+ private final ConfigService configService;
public BanyanDBIndexInstaller(Client client, ModuleManager moduleManager, BanyanDBStorageConfig config) {
super(client, moduleManager);
this.config = config;
+ this.configService = moduleManager.find(CoreModule.NAME).provider().getService(ConfigService.class);
}
@Override
protected boolean isExists(Model model) throws StorageException {
- final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config);
+ final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config, configService);
try {
final BanyanDBClient c = ((BanyanDBStorageClient) this.client).client;
- // first check group
- Group g = metadata.getOrCreateGroup(c);
- if (g == null) {
- throw new StorageException("fail to create group " + metadata.getGroup());
+ // first check resource existence and create group if necessary
+ final boolean resourceExist = metadata.checkResourceExistence(c);
+ if (!resourceExist) {
+ return false;
}
- log.info("group {} created", g.name());
+
// then check entity schema
if (metadata.findRemoteSchema(c).isPresent()) {
- MetadataRegistry.INSTANCE.registerModel(model, config);
+ MetadataRegistry.INSTANCE.registerModel(model, config, configService);
Review Comment:
Since I've removed this `registerModel` action here, it is now simplified
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011804989
##########
oap-server/server-starter/src/main/resources/application.yml:
##########
@@ -236,6 +236,10 @@ storage:
superDatasetShardsFactor: ${SW_STORAGE_BANYANDB_SUPERDATASET_SHARDS_FACTOR:2}
concurrentWriteThreads: ${SW_STORAGE_BANYANDB_CONCURRENT_WRITE_THREADS:15}
profileTaskQueryMaxSize: ${SW_STORAGE_BANYANDB_PROFILE_TASK_QUERY_MAX_SIZE:200} # the max number of fetch task in a request
+ streamBlockInterval: ${SW_STORAGE_BANYANDB_STREAM_BLOCK_INTERVAL:4} # Unit is Hours
+ streamSegmentInterval: ${SW_STORAGE_BANYANDB_STREAM_SEGMENT_INTERVAL:24} # Unit is Hours
+ measureBlockInterval: ${SW_STORAGE_BANYANDB_MEASURE_BLOCK_INTERVAL:4} # Unit is Hours
+ measureSegmentInterval: ${SW_STORAGE_BANYANDB_MEASURE_SEGMENT_INTERVAL:24} # Unit is Hours
Review Comment:
backend-storage.md should be updated as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1012434181
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBIndexInstaller.java:
##########
@@ -35,28 +36,32 @@
@Slf4j
public class BanyanDBIndexInstaller extends ModelInstaller {
private final BanyanDBStorageConfig config;
+ private final ConfigService configService;
public BanyanDBIndexInstaller(Client client, ModuleManager moduleManager, BanyanDBStorageConfig config) {
super(client, moduleManager);
this.config = config;
+ this.configService = moduleManager.find(CoreModule.NAME).provider().getService(ConfigService.class);
}
@Override
protected boolean isExists(Model model) throws StorageException {
- final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config);
+ final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config, configService);
try {
final BanyanDBClient c = ((BanyanDBStorageClient) this.client).client;
- // first check group
- Group g = metadata.getOrCreateGroup(c);
- if (g == null) {
- throw new StorageException("fail to create group " + metadata.getGroup());
+ // first check resource existence and create group if necessary
+ final boolean resourceExist = metadata.checkResourceExistence(c);
+ if (!resourceExist) {
+ return false;
}
- log.info("group {} created", g.name());
+
// then check entity schema
if (metadata.findRemoteSchema(c).isPresent()) {
- MetadataRegistry.INSTANCE.registerModel(model, config);
+ MetadataRegistry.INSTANCE.registerModel(model, config, configService);
Review Comment:
Got it. My fault. we should be good now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011973110
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/measure/BanyanDBMetricsQueryDAO.java:
##########
@@ -205,19 +199,38 @@ public HeatMap readHeatMap(MetricsCondition condition, String valueColumnName, D
return heatMap;
}
- private Map<String, DataPoint> queryIDs(String modelName, String valueColumnName, List<String> measureIDs) throws IOException {
- Map<String, DataPoint> map = new HashMap<>(measureIDs.size());
- for (final String id : measureIDs) {
- MeasureQueryResponse resp = query(modelName, Collections.emptySet(), ImmutableSet.of(valueColumnName), new QueryBuilder<MeasureQuery>() {
- @Override
- protected void apply(MeasureQuery query) {
- query.andWithID(id);
- }
- });
- if (resp.size() > 0) {
- map.putIfAbsent(resp.getDataPoints().get(0).getId(), resp.getDataPoints().get(0));
+ private List<String> extractMeasureIDs(Duration duration, String entityID) {
+ final List<PointOfTime> pointOfTimes = duration.assembleDurationPoints();
+ List<String> ids = new ArrayList<>(pointOfTimes.size());
+ pointOfTimes.forEach(pointOfTime -> {
+ String id = pointOfTime.id(entityID);
+ ids.add(id);
+ });
+ return ids;
+ }
+
+ private Map<String, DataPoint> queryByEntityID(final MetricsCondition condition, String valueColumnName, Duration duration) throws IOException {
+ final MetadataRegistry.Schema schema = MetadataRegistry.INSTANCE.findMetadata(condition.getName(), duration.getStep());
+ if (schema == null) {
+ throw new IOException("schema is not registered");
+ }
+ return queryByEntityID(schema, valueColumnName, duration, condition.getEntity().buildId());
+ }
+
+ private Map<String, DataPoint> queryByEntityID(MetadataRegistry.Schema schema, String valueColumnName, Duration duration, String entityID) throws IOException {
+ TimestampRange timestampRange = new TimestampRange(duration.getStartTimestamp(), duration.getEndTimestamp());
+
+ Map<String, DataPoint> map = new HashMap<>();
+ MeasureQueryResponse resp = query(schema, Collections.emptySet(), ImmutableSet.of(valueColumnName), timestampRange, new QueryBuilder<MeasureQuery>() {
+ @Override
+ protected void apply(MeasureQuery query) {
+ query.and(eq(Metrics.ENTITY_ID, entityID));
}
+ });
+ for (final DataPoint dp : resp.getDataPoints()) {
+ map.putIfAbsent(dp.getId(), dp);
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#issuecomment-1298804715
According to the discussion between @hanahmily and me, we missed downsampling based index mechanism from v0.1.
We should fix this to make sure time dimension wouldn't be mixed.For example, 5pm has 3 values in three dimensions to represent precisions. They are mixed for now.
@lujiajing1126 please correct me if I am wrong.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] hanahmily commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
hanahmily commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011241170
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/measure/BanyanDBMetricsQueryDAO.java:
##########
@@ -205,19 +199,38 @@ public HeatMap readHeatMap(MetricsCondition condition, String valueColumnName, D
return heatMap;
}
- private Map<String, DataPoint> queryIDs(String modelName, String valueColumnName, List<String> measureIDs) throws IOException {
- Map<String, DataPoint> map = new HashMap<>(measureIDs.size());
- for (final String id : measureIDs) {
- MeasureQueryResponse resp = query(modelName, Collections.emptySet(), ImmutableSet.of(valueColumnName), new QueryBuilder<MeasureQuery>() {
- @Override
- protected void apply(MeasureQuery query) {
- query.andWithID(id);
- }
- });
- if (resp.size() > 0) {
- map.putIfAbsent(resp.getDataPoints().get(0).getId(), resp.getDataPoints().get(0));
+ private List<String> extractMeasureIDs(Duration duration, String entityID) {
+ final List<PointOfTime> pointOfTimes = duration.assembleDurationPoints();
+ List<String> ids = new ArrayList<>(pointOfTimes.size());
+ pointOfTimes.forEach(pointOfTime -> {
+ String id = pointOfTime.id(entityID);
+ ids.add(id);
+ });
+ return ids;
+ }
+
+ private Map<String, DataPoint> queryByEntityID(final MetricsCondition condition, String valueColumnName, Duration duration) throws IOException {
+ final MetadataRegistry.Schema schema = MetadataRegistry.INSTANCE.findMetadata(condition.getName(), duration.getStep());
+ if (schema == null) {
+ throw new IOException("schema is not registered");
+ }
+ return queryByEntityID(schema, valueColumnName, duration, condition.getEntity().buildId());
+ }
+
+ private Map<String, DataPoint> queryByEntityID(MetadataRegistry.Schema schema, String valueColumnName, Duration duration, String entityID) throws IOException {
+ TimestampRange timestampRange = new TimestampRange(duration.getStartTimestamp(), duration.getEndTimestamp());
+
+ Map<String, DataPoint> map = new HashMap<>();
+ MeasureQueryResponse resp = query(schema, Collections.emptySet(), ImmutableSet.of(valueColumnName), timestampRange, new QueryBuilder<MeasureQuery>() {
+ @Override
+ protected void apply(MeasureQuery query) {
+ query.and(eq(Metrics.ENTITY_ID, entityID));
}
+ });
+ for (final DataPoint dp : resp.getDataPoints()) {
+ map.putIfAbsent(dp.getId(), dp);
Review Comment:
If a duplicated data point is here, please raise warnings to make some noise.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] hanahmily commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
hanahmily commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011141069
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/MetadataRegistry.java:
##########
@@ -293,21 +335,41 @@ public SchemaMetadata parseMetadata(Model model, BanyanDBStorageConfig config) {
@Data
public static class SchemaMetadata {
private final String group;
- private final String name;
+ /**
+ * name of the {@link Model}
+ */
+ private final String modelName;
private final Kind kind;
-
+ /**
+ * down-sampling of the {@link Model}
+ */
+ private final DownSampling downSampling;
private final int shard;
private final int blockIntervalHrs;
private final int segmentIntervalHrs;
private final int ttlDays;
+ /**
+ * Format the entity name for BanyanDB
+ *
+ * @param modelName name of the model
+ * @param downSampling not used if it is {@link DownSampling#None}
+ * @return entity (e.g. measure, stream) name
+ */
+ static String formatName(String modelName, DownSampling downSampling) {
+ if (downSampling == null || downSampling == DownSampling.None) {
+ return modelName;
+ }
+ return modelName + ":" + downSampling.getName();
Review Comment:
Underscore "_" is better and compatible with the metric name's convention.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1012418903
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBIndexInstaller.java:
##########
@@ -35,28 +36,32 @@
@Slf4j
public class BanyanDBIndexInstaller extends ModelInstaller {
private final BanyanDBStorageConfig config;
+ private final ConfigService configService;
public BanyanDBIndexInstaller(Client client, ModuleManager moduleManager, BanyanDBStorageConfig config) {
super(client, moduleManager);
this.config = config;
+ this.configService = moduleManager.find(CoreModule.NAME).provider().getService(ConfigService.class);
}
@Override
protected boolean isExists(Model model) throws StorageException {
- final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config);
+ final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config, configService);
try {
final BanyanDBClient c = ((BanyanDBStorageClient) this.client).client;
- // first check group
- Group g = metadata.getOrCreateGroup(c);
- if (g == null) {
- throw new StorageException("fail to create group " + metadata.getGroup());
+ // first check resource existence and create group if necessary
+ final boolean resourceExist = metadata.checkResourceExistence(c);
+ if (!resourceExist) {
+ return false;
}
- log.info("group {} created", g.name());
+
// then check entity schema
if (metadata.findRemoteSchema(c).isPresent()) {
- MetadataRegistry.INSTANCE.registerModel(model, config);
+ MetadataRegistry.INSTANCE.registerModel(model, config, configService);
Review Comment:
> Do we have a better to make codes more clear? Such as, when we do `isPresent`, could we use the metadata of existing measure/stream to build this local registration automatically?
I don't think it is enough to extract all necessary metadata with the schema in the BanyanDB. For example,
```java
public static class StreamToStorage implements Convert2Storage<StreamWrite> {
...
@Override
public void accept(String fieldName, Object fieldValue) {
MetadataRegistry.ColumnSpec columnSpec = this.schema.getSpec(fieldName);
if (columnSpec == null) {
throw new IllegalArgumentException("fail to find field[" + fieldName + "]");
}
try {
this.streamWrite.tag(fieldName, buildTag(fieldValue, columnSpec.getColumnClass()));
} catch (BanyanDBException ex) {
log.error("fail to add tag", ex);
}
}
}
private static Serializable<BanyandbModel.TagValue> buildTag(Object value, final Class<?> clazz) {
if (int.class.equals(clazz) || Integer.class.equals(clazz)) {
return TagAndValue.longTagValue(((Number) value).longValue());
} else if (Long.class.equals(clazz) || long.class.equals(clazz)) {
return TagAndValue.longTagValue((Long) value);
} else if (String.class.equals(clazz)) {
return TagAndValue.stringTagValue((String) value);
} else if (Double.class.equals(clazz) || double.class.equals(clazz)) {
return TagAndValue.binaryTagValue(ByteUtil.double2Bytes((double) value));
} else if (StorageDataComplexObject.class.isAssignableFrom(clazz)) {
return TagAndValue.stringTagValue(((StorageDataComplexObject<?>) value).toStorageData());
} else if (clazz.isEnum()) {
return TagAndValue.longTagValue((int) value);
} else if (JsonObject.class.equals(clazz)) {
return TagAndValue.stringTagValue((String) value);
} else if (byte[].class.equals(clazz)) {
return TagAndValue.stringTagValue((String) value);
}
throw new IllegalStateException(clazz.getSimpleName() + " is not supported");
}
```
The mapping between data types in BanyanDB and OAP is not a surjection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1012419443
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBIndexInstaller.java:
##########
@@ -35,28 +36,32 @@
@Slf4j
public class BanyanDBIndexInstaller extends ModelInstaller {
private final BanyanDBStorageConfig config;
+ private final ConfigService configService;
public BanyanDBIndexInstaller(Client client, ModuleManager moduleManager, BanyanDBStorageConfig config) {
super(client, moduleManager);
this.config = config;
+ this.configService = moduleManager.find(CoreModule.NAME).provider().getService(ConfigService.class);
}
@Override
protected boolean isExists(Model model) throws StorageException {
- final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config);
+ final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config, configService);
try {
final BanyanDBClient c = ((BanyanDBStorageClient) this.client).client;
- // first check group
- Group g = metadata.getOrCreateGroup(c);
- if (g == null) {
- throw new StorageException("fail to create group " + metadata.getGroup());
+ // first check resource existence and create group if necessary
+ final boolean resourceExist = metadata.checkResourceExistence(c);
+ if (!resourceExist) {
+ return false;
}
- log.info("group {} created", g.name());
+
// then check entity schema
if (metadata.findRemoteSchema(c).isPresent()) {
- MetadataRegistry.INSTANCE.registerModel(model, config);
+ MetadataRegistry.INSTANCE.registerModel(model, config, configService);
Review Comment:
I see, then how about we separating local registration logic as new methods?
Remote registration could call local registration automatically for sure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 commented on pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#issuecomment-1296442141
> @kezhenxu94 skywalking-eye fails to handle some repos' license:
>
> ```
> ERROR failed to identify the licenses of following packages (2):
> build.buf.protoc-gen-validate:pgv-java-stub
> build.buf.protoc-gen-validate:protoc-gen-validate
> Error: Process completed with exit code 1.
> ```
>
> https://github.com/apache/skywalking/actions/runs/3342606435/jobs/5537555471
There is no licensing information from the perspective of maven dependency, so for now I'm afraid you have to apply the patch:
```diff
diff --git a/.licenserc.yaml b/.licenserc.yaml
index e7a33ff930..ec0af00052 100644
--- a/.licenserc.yaml
+++ b/.licenserc.yaml
@@ -119,3 +119,9 @@ dependency:
- name: com.google.flatbuffers:flatbuffers-java
version: 1.12.0
license: Apache-2.0
+ - name: build.buf.protoc-gen-validate:pgv-java-stub
+ version: 0.6.13
+ license: Apache-2.0
+ - name: build.buf.protoc-gen-validate:protoc-gen-validate
+ version: 0.6.13
+ license: Apache-2.0
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011820521
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBIndexInstaller.java:
##########
@@ -35,28 +36,32 @@
@Slf4j
public class BanyanDBIndexInstaller extends ModelInstaller {
private final BanyanDBStorageConfig config;
+ private final ConfigService configService;
public BanyanDBIndexInstaller(Client client, ModuleManager moduleManager, BanyanDBStorageConfig config) {
super(client, moduleManager);
this.config = config;
+ this.configService = moduleManager.find(CoreModule.NAME).provider().getService(ConfigService.class);
}
@Override
protected boolean isExists(Model model) throws StorageException {
- final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config);
+ final MetadataRegistry.SchemaMetadata metadata = MetadataRegistry.INSTANCE.parseMetadata(model, config, configService);
try {
final BanyanDBClient c = ((BanyanDBStorageClient) this.client).client;
- // first check group
- Group g = metadata.getOrCreateGroup(c);
- if (g == null) {
- throw new StorageException("fail to create group " + metadata.getGroup());
+ // first check resource existence and create group if necessary
+ final boolean resourceExist = metadata.checkResourceExistence(c);
+ if (!resourceExist) {
+ return false;
}
- log.info("group {} created", g.name());
+
// then check entity schema
if (metadata.findRemoteSchema(c).isPresent()) {
- MetadataRegistry.INSTANCE.registerModel(model, config);
+ MetadataRegistry.INSTANCE.registerModel(model, config, configService);
Review Comment:
I think we should keep existence checking here only. Otherwise, what is the point of `createTable`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011825987
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBIndexInstaller.java:
##########
@@ -67,13 +72,13 @@ protected boolean isExists(Model model) throws StorageException {
protected void createTable(Model model) throws StorageException {
try {
if (model.isTimeSeries() && model.isRecord()) { // stream
- Stream stream = (Stream) MetadataRegistry.INSTANCE.registerModel(model, config);
+ Stream stream = (Stream) MetadataRegistry.INSTANCE.registerModel(model, config, configService);
if (stream != null) {
log.info("install stream schema {}", model.getName());
((BanyanDBStorageClient) client).define(stream);
}
} else if (model.isTimeSeries() && !model.isRecord()) { // measure
- Measure measure = (Measure) MetadataRegistry.INSTANCE.registerModel(model, config);
+ Measure measure = (Measure) MetadataRegistry.INSTANCE.registerModel(model, config, configService);
Review Comment:
I think we should separate `registerStreamModel` and `registerMeasureModel` to keep API clear, especially for the `return type`.
And where would we create the object when `!model.isTimeSeries()`? Or it is not required to be pre-created?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011814212
##########
oap-server/server-starter/src/main/resources/application.yml:
##########
@@ -236,6 +236,10 @@ storage:
superDatasetShardsFactor: ${SW_STORAGE_BANYANDB_SUPERDATASET_SHARDS_FACTOR:2}
concurrentWriteThreads: ${SW_STORAGE_BANYANDB_CONCURRENT_WRITE_THREADS:15}
profileTaskQueryMaxSize: ${SW_STORAGE_BANYANDB_PROFILE_TASK_QUERY_MAX_SIZE:200} # the max number of fetch task in a request
+ streamBlockInterval: ${SW_STORAGE_BANYANDB_STREAM_BLOCK_INTERVAL:4} # Unit is Hours
+ streamSegmentInterval: ${SW_STORAGE_BANYANDB_STREAM_SEGMENT_INTERVAL:24} # Unit is Hours
+ measureBlockInterval: ${SW_STORAGE_BANYANDB_MEASURE_BLOCK_INTERVAL:4} # Unit is Hours
+ measureSegmentInterval: ${SW_STORAGE_BANYANDB_MEASURE_SEGMENT_INTERVAL:24} # Unit is Hours
Review Comment:
```suggestion
streamBlockInterval: ${SW_STORAGE_BANYANDB_STREAM_BLOCK_INTERVAL:4} # Unit is hour
streamSegmentInterval: ${SW_STORAGE_BANYANDB_STREAM_SEGMENT_INTERVAL:24} # Unit is hour
measureBlockInterval: ${SW_STORAGE_BANYANDB_MEASURE_BLOCK_INTERVAL:4} # Unit is hour
measureSegmentInterval: ${SW_STORAGE_BANYANDB_MEASURE_SEGMENT_INTERVAL:24} # Unit is hour
```
H should not be in upper case, and don't need `s`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011247886
##########
oap-server-bom/pom.xml:
##########
@@ -73,7 +73,7 @@
<awaitility.version>3.0.0</awaitility.version>
<httpcore.version>4.4.13</httpcore.version>
<commons-compress.version>1.21</commons-compress.version>
- <banyandb-java-client.version>0.1.0</banyandb-java-client.version>
+ <banyandb-java-client.version>0.2.0-SNAPSHOT</banyandb-java-client.version>
Review Comment:
TODO: Once PR is approved, we would wait for the BanyanDB client 0.2.0 released.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011297180
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBConverter.java:
##########
@@ -215,11 +218,19 @@ public static class StorageToMeasure implements Convert2Entity {
private final MetadataRegistry.Schema schema;
private final DataPoint dataPoint;
- public StorageToMeasure(String modelName, DataPoint dataPoint) {
- this.schema = MetadataRegistry.INSTANCE.findMetadata(modelName);
+ public StorageToMeasure(MetadataRegistry.Schema schema, DataPoint dataPoint) {
+ this.schema = schema;
this.dataPoint = dataPoint;
}
+ /**
+ * Currently, only used by {@link org.apache.skywalking.oap.server.storage.plugin.banyandb.measure.BanyanDBNetworkAddressAliasDAO}.
+ * The default DownSampling strategy, i.e. {@link DownSampling#Minute} is assumed in this case.
+ */
+ public StorageToMeasure(String measureModelName, DataPoint dataPoint) {
+ this(checkNotNull(MetadataRegistry.INSTANCE.findMetadata(measureModelName, DownSampling.Minute), "measure schema"), dataPoint);
Review Comment:
What assumption? Could you be more specific?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011296313
##########
oap-server/server-starter/src/main/resources/application.yml:
##########
@@ -236,6 +236,12 @@ storage:
superDatasetShardsFactor: ${SW_STORAGE_BANYANDB_SUPERDATASET_SHARDS_FACTOR:2}
concurrentWriteThreads: ${SW_STORAGE_BANYANDB_CONCURRENT_WRITE_THREADS:15}
profileTaskQueryMaxSize: ${SW_STORAGE_BANYANDB_PROFILE_TASK_QUERY_MAX_SIZE:200} # the max number of fetch task in a request
+ streamBlockInterval: ${SW_STORAGE_BANYANDB_STREAM_BLOCK_INTERVAL:4} # Unit is Hours
+ streamSegmentInterval: ${SW_STORAGE_BANYANDB_STREAM_SEGMENT_INTERVAL:24} # Unit is Hours
+ streamTTL: ${SW_STORAGE_BANYANDB_STREAM_TTL:24} # Unit is Days
Review Comment:
Yes, `ConfigService` is exposed from the Core module.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011303190
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBConverter.java:
##########
@@ -215,11 +218,19 @@ public static class StorageToMeasure implements Convert2Entity {
private final MetadataRegistry.Schema schema;
private final DataPoint dataPoint;
- public StorageToMeasure(String modelName, DataPoint dataPoint) {
- this.schema = MetadataRegistry.INSTANCE.findMetadata(modelName);
+ public StorageToMeasure(MetadataRegistry.Schema schema, DataPoint dataPoint) {
+ this.schema = schema;
this.dataPoint = dataPoint;
}
+ /**
+ * Currently, only used by {@link org.apache.skywalking.oap.server.storage.plugin.banyandb.measure.BanyanDBNetworkAddressAliasDAO}.
+ * The default DownSampling strategy, i.e. {@link DownSampling#Minute} is assumed in this case.
+ */
+ public StorageToMeasure(String measureModelName, DataPoint dataPoint) {
+ this(checkNotNull(MetadataRegistry.INSTANCE.findMetadata(measureModelName, DownSampling.Minute), "measure schema"), dataPoint);
Review Comment:
> What assumption? Could you be more specific?
Does this mean `DownSampling == Minute`? If so, for this specific case, yes, it is correct. `NetworkAddressAlias` has this annotation `@MetricsExtension(supportDownSampling = false, supportUpdate = true)`.
`*Traffic` has the same annotation for no downsampling too. This is a very special annotation, for now, it is used for models having their own DAOs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1011772285
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBConverter.java:
##########
@@ -215,11 +218,19 @@ public static class StorageToMeasure implements Convert2Entity {
private final MetadataRegistry.Schema schema;
private final DataPoint dataPoint;
- public StorageToMeasure(String modelName, DataPoint dataPoint) {
- this.schema = MetadataRegistry.INSTANCE.findMetadata(modelName);
+ public StorageToMeasure(MetadataRegistry.Schema schema, DataPoint dataPoint) {
+ this.schema = schema;
this.dataPoint = dataPoint;
}
+ /**
+ * Currently, only used by {@link org.apache.skywalking.oap.server.storage.plugin.banyandb.measure.BanyanDBNetworkAddressAliasDAO}.
+ * The default DownSampling strategy, i.e. {@link DownSampling#Minute} is assumed in this case.
+ */
+ public StorageToMeasure(String measureModelName, DataPoint dataPoint) {
+ this(checkNotNull(MetadataRegistry.INSTANCE.findMetadata(measureModelName, DownSampling.Minute), "measure schema"), dataPoint);
Review Comment:
Moved to `BanyanDBNetworkAddressAliasDAO`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] lujiajing1126 commented on a diff in pull request #9839: Update BanyanDB storage module to adapt BanyanDB 0.2
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #9839:
URL: https://github.com/apache/skywalking/pull/9839#discussion_r1007577173
##########
oap-server-bom/pom.xml:
##########
@@ -73,7 +73,7 @@
<awaitility.version>3.0.0</awaitility.version>
<httpcore.version>4.4.13</httpcore.version>
<commons-compress.version>1.21</commons-compress.version>
- <banyandb-java-client.version>0.1.0</banyandb-java-client.version>
+ <banyandb-java-client.version>0.2.0-SNAPSHOT</banyandb-java-client.version>
Review Comment:
Yes. Pending util we publish java client `0.2.0`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org