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 2021/04/11 15:46:52 UTC
[GitHub] [skywalking] nickwongwong opened a new issue #6728: GlobalIdGenerator will product negative id when time shift back
nickwongwong opened a new issue #6728:
URL: https://github.com/apache/skywalking/issues/6728
Please answer these questions before submitting your issue.
- Why do you submit this issue?
- [ ] Question or discussion
- [ ] Bug
- [ ] Requirement
- [x] Feature or performance improvement
___
### Requirement or improvement
Which version of SkyWalking, OS, and JRE?
8.4.0 MacOS JDK8
- Please describe your requirements or improvement suggestions.
Random will return a negative value, while this is an easy way to solve this.
https://github.com/apache/skywalking/blob/f3ca0d0ea52bf05df87e9b1e268b7f5d69423a88/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/ids/GlobalIdGenerator.java#L78
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] nickwongwong commented on issue #6728: GlobalIdGenerator will produce a negative id when the time shift back
Posted by GitBox <gi...@apache.org>.
nickwongwong commented on issue #6728:
URL: https://github.com/apache/skywalking/issues/6728#issuecomment-817333178
Relative PR:
https://github.com/apache/skywalking/pull/307#issuecomment-316992773
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] nickwongwong commented on issue #6728: GlobalIdGenerator will produce a negative id when the time shift back
Posted by GitBox <gi...@apache.org>.
nickwongwong commented on issue #6728:
URL: https://github.com/apache/skywalking/issues/6728#issuecomment-817335610
It may produce a conflict timestamp while using random.
I have a new solution for the time-shift-back, inspiring by `sonyflake`.
https://github.com/sony/sonyflake/blob/848d664ceea4c980874f2135c85c42409c530b1f/sonyflake.go#L96-L103
After considering performance and safety, sleep some ms when `overtime` is less than 5ms.
Use random integer when `overtime` is more than 5ms.
Please review my new PR.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #6728: GlobalIdGenerator will produce a negative id when the time shift back
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #6728:
URL: https://github.com/apache/skywalking/issues/6728#issuecomment-817330169
How does this matter? We don't say it has to be positive.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] nickwongwong commented on issue #6728: GlobalIdGenerator will produce a negative id when the time shift back
Posted by GitBox <gi...@apache.org>.
nickwongwong commented on issue #6728:
URL: https://github.com/apache/skywalking/issues/6728#issuecomment-817349195
> > It may be confusing and ugly.
>
> I am not sure what is confusing. Every agent could use a different trace id generating rule. Java is complex one, others use maybe UUID directly.
>
> > It may produce a conflict timestamp while using random.
>
> If we are going to discuss conflict, then, I would say negative is even better? Because we would have a negative value in a normal case.
>
> > I have a new solution for the time-shift-back, inspiring by sonyflake.
>
> I don't think we need to resolve this complex issue. And more importantly, why are you doing time shifting? Or it is just because of OS timestamp adjusting?
我们的目标先统一一下：在保证性能的前提下，使`globalId`的全局唯一
Our goal is to unify first: under the premise of ensuring performance, make the `globalId` globally unique
> It may be confusing and ugly.
文档中说明是用`timestamp`, 显然`timestamp`不可能是负数的。
函数名是`nextSeq`, `sequence` 作为负数也很奇怪。
另外“-”在很多情况下被当做是分隔符，突然在这里出现会很奇怪
The document states that `timestamp` is used. Obviously, `timestamp` cannot be a negative number.
The function name is `nextSeq`, and it is strange that `sequence` is a negative number.
In addition, "-" is used as a separator in many cases, it would be strange to suddenly appear here
> It may produce a conflict timestamp while using random.
> If we are going to discuss conflict, then, I would say negative is even better? Because we would have a negative value in a normal case.
如果时间回拨多次，使用随机数，这有一定的概率会冲突(虽然很小).
现在的时间戳是1618164032218，已经超出了Integer的范围，所以随机数 `nextInt(0, Integer.MAX_VALUE)` 不会和正常的时间戳冲突.
If the time is called back multiple times and a random number is used, there is a certain probability that it will conflict (albeit very small).
The current timestamp is 1618164032218, which has exceeded the range of Integer, so the random number `nextInt(0, Integer.MAX_VALUE)` will not conflict with the normal timestamp.
> why are you doing time shifting? Or it is just because OS timestamp adjusting?
OS时间变更或者时间校对是很难完全避免的。既然我们代码里面处理了时间回拨问题，是可以做进一步的优化的。
It is difficult to completely avoid OS time changes or time proofreading. Since our code handles the time-callback problem, it can be further optimized.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #6728: GlobalIdGenerator will produce a negative id when the time shift back
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #6728:
URL: https://github.com/apache/skywalking/issues/6728#issuecomment-817337184
> It may confusing and ugly.
I am not sure what is confusing. Every agent could use different trace id generating rule. Java is complex one, others use maybe UUID directly.
> It may produce a conflict timestamp while using random.
If we are going to discuss conflict, then, I would say negative is even better? Because we would have a negative value in a normal case.
> I have a new solution for the time-shift-back, inspiring by sonyflake.
I don't think we need to resolve this complex issue. And more importantly, why are you doing time shifting? Or it is just because OS timestamp adjusting?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] nickwongwong commented on issue #6728: GlobalIdGenerator will produce a negative id when the time shift back
Posted by GitBox <gi...@apache.org>.
nickwongwong commented on issue #6728:
URL: https://github.com/apache/skywalking/issues/6728#issuecomment-817331458
It will make a PR and talk about the detail later
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #6728: GlobalIdGenerator will produce a negative id when the time shift back
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #6728:
URL: https://github.com/apache/skywalking/issues/6728#issuecomment-817413037
> The document states that timestamp is used. Obviously, timestamp cannot be a negative number.
The function name is nextSeq, and it is strange that sequence is a negative number.
In addition, "-" is used as a separator in many cases, it would be strange to suddenly appear here
Yes and no. Time shift is a very abnormal state. We don't expect this happens in a normal case, that is why I am asking.
Your change is acceptable, but also I want to make sure, you are not running into a wrong direction.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] nickwongwong edited a comment on issue #6728: GlobalIdGenerator will produce a negative id when the time shift back
Posted by GitBox <gi...@apache.org>.
nickwongwong edited a comment on issue #6728:
URL: https://github.com/apache/skywalking/issues/6728#issuecomment-817332841
A Normal GlobalID:
`9d79f630589846e7a781e1f2fc11039e.340.16181571607822794`
While with negative sign:
`9d79f630589846e7a781e1f2fc11039e.340.-16181571607822794`
It may confusing and strange.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] nickwongwong commented on issue #6728: GlobalIdGenerator will produce a negative id when the time shift back
Posted by GitBox <gi...@apache.org>.
nickwongwong commented on issue #6728:
URL: https://github.com/apache/skywalking/issues/6728#issuecomment-817332841
A Normal GlobalID:
`9d79f630589846e7a781e1f2fc11039e.340.16181571607822794`
While with negative sign:
`9d79f630589846e7a781e1f2fc11039e.340.-16181571607822794`
It may confusing and ugly.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng closed issue #6728: GlobalIdGenerator will produce a negative id when the time shift back
Posted by GitBox <gi...@apache.org>.
wu-sheng closed issue #6728:
URL: https://github.com/apache/skywalking/issues/6728
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org