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/11/29 11:19:45 UTC

[GitHub] [skywalking] mrproliu opened a new pull request, #10049: Fix wrong time bucket on attached event

mrproliu opened a new pull request, #10049:
URL: https://github.com/apache/skywalking/pull/10049

   Fix the wrong time bucket on the attached event, otherwise, the event will write to the monthly index in ES storage.
   
   - [ ] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #<issue number>.
   - [ ] 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] wankai123 commented on pull request #10049: Fix wrong time bucket on attached event

Posted by GitBox <gi...@apache.org>.
wankai123 commented on PR #10049:
URL: https://github.com/apache/skywalking/pull/10049#issuecomment-1330520741

   > > I think this can't be read if the time bucket is wrong, right?
   > 
   > No, I have done the code research. When we query the records, we just use the alias name, such as `records-all`, the wrong index name will be `records-all-YYYYMM`. But for the long term, the data cannot be queried, because we have the `TTL` mechanism to delete the wrong and older indexes. In the E2E, unless we query it for a long time, so we cannot find this kind of problem.
   
   I think we should query like this:
   ```java
           final SearchResponse response = getClient().search(
               new TimeRangeIndexNameGenerator(
                   IndexController.LogicIndicesRegister.getPhysicalTableName(SegmentRecord.INDEX_NAME),
                   startSecondTB,
                   endSecondTB
               ), search.build());
   ```


-- 
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] mrproliu commented on pull request #10049: Fix wrong time bucket on attached event

Posted by GitBox <gi...@apache.org>.
mrproliu commented on PR #10049:
URL: https://github.com/apache/skywalking/pull/10049#issuecomment-1330481529

   > This seems a thing could be covered by e2e.
   
   Do you mean using e2e to check the index name or time bucket value?
   


-- 
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 merged pull request #10049: Fix wrong time bucket on attached event

Posted by GitBox <gi...@apache.org>.
wu-sheng merged PR #10049:
URL: https://github.com/apache/skywalking/pull/10049


-- 
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] mrproliu commented on pull request #10049: Fix wrong time bucket on attached event

Posted by GitBox <gi...@apache.org>.
mrproliu commented on PR #10049:
URL: https://github.com/apache/skywalking/pull/10049#issuecomment-1330541547

   Sure, but for the attached event query, we need to calculate all of the spans to get the start/end time. If the event timestamp does not contain in the span timestamp range(such as has offset between span and event), It may not be queried in some extreme situations. 


-- 
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 #10049: Fix wrong time bucket on attached event

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #10049:
URL: https://github.com/apache/skywalking/pull/10049#issuecomment-1330474734

   This seems a thing could be covered by e2e.


-- 
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 #10049: Fix wrong time bucket on attached event

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #10049:
URL: https://github.com/apache/skywalking/pull/10049#issuecomment-1330568139

   For `SampledStatus4xxTraceRecord` and others, please move `@BanyanDB.SeriesID(index = 0)` to entity ID.


-- 
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 #10049: Fix wrong time bucket on attached event

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #10049:
URL: https://github.com/apache/skywalking/pull/10049#issuecomment-1330525446

   Yes, we should use physical indices to query, it has much better performance.


-- 
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] mrproliu commented on pull request #10049: Fix wrong time bucket on attached event

Posted by GitBox <gi...@apache.org>.
mrproliu commented on PR #10049:
URL: https://github.com/apache/skywalking/pull/10049#issuecomment-1330515239

   > I think this can't be read if the time bucket is wrong, right?
   
   No, I have done the code research. When we query the records, we just use the alias name, such as `records-all`, the wrong index name will be `records-all-YYYYMM`. But for the long term, the data cannot be queried, because we have the `TTL` mechanism to delete the wrong and older indexes. 
   In the E2E, unless we query it for a long time, so we cannot find this kind of problem.


-- 
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 #10049: Fix wrong time bucket on attached event

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #10049:
URL: https://github.com/apache/skywalking/pull/10049#issuecomment-1330484159

   I think this can't be read if the time bucket is wrong, 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] wu-sheng commented on pull request #10049: Fix wrong time bucket on attached event

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #10049:
URL: https://github.com/apache/skywalking/pull/10049#issuecomment-1330750576

   ```
   Caused by: java.lang.IllegalArgumentException: ColumnName(modelName=top_n_cache_read_command, fullName=trace_id, storageName=trace_id) and ColumnName(modelName=sampled_slow_trace_record, fullName=trace_id, storageName=trace_id) isStorageOnly conflict in index: records-all
   ```
   
   Let's review all not `superset` records, and whether they all should be `storageOnly = true`. I think there are several other records conflicting with 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 pull request #10049: Fix wrong time bucket on attached event

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on PR #10049:
URL: https://github.com/apache/skywalking/pull/10049#issuecomment-1330551299

   You reminded me, I think there are some things wrong with the SpanAttachedEventRecord annotation about BanyanDB.
   
   Please use `@BanyanDB.GlobalIndex` for the trace ID. And we need to think about the `@SeriesID`
   
   


-- 
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