Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

[Producer] Change the time units from ns to ms #13057

Merged
merged 2 commits into from
Dec 23, 2021
Merged

[Producer] Change the time units from ns to ms #13057

merged 2 commits into from
Dec 23, 2021

Conversation

tomscut
Copy link
Contributor

@tomscut tomscut commented Nov 30, 2021

Motivation

The time unit in this exception message is ns, which is not very readable. We can change it from ns to ms.

org.apache.pulsar.client.api.PulsarClientException$TimeoutException:
The producer xxx can not send message to the topic xxx within given timeout : createdAt 461913074 ns ago, firstSentAt 29545553038276935 ns ago, lastSentAt 29545553038276935 ns ago, retryCount 0 at org.apache.pulsar.client.api.PulsarClientException.unwrap(PulsarClientException.java:916)
at org.apache.pulsar.client.impl.TypedMessageBuilderImpl.send(TypedMessageBuilderImpl.java:93)
at org.apache.pulsar.client.impl.ProducerBase.send(ProducerBase.java:63)
at com.yum.boh.oh.service.impl.StoreOrderPostServiceImpl.generalProcessing(StoreOrderPostServiceImpl.java:272)
at com.yum.boh.oh.service.impl.StoreOrderPostServiceImpl.saveThirdOrder(StoreOrderPostServiceImpl.java:72)
at com.yum.boh.oh.controller.StoreOrderController.postOrderInfo$original$T8425mfx(StoreOrderController.java:39)
at com.yum.boh.oh.controller.StoreOrderController.postOrderInfo$original$T8425mfx$accessor$vJljNzML(StoreOrderController.java)
at com.yum.boh.oh.controller.StoreOrderController$auxiliary$nysalhgy.call(Unknown Source)
at org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstMethodsInter.intercept(InstMethodsInter.java:86)
...

Modifications

Change the time units from ns to ms for ProducerImpl#OpSendMsg.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

  • doc-required
  • no-need-doc
  • doc

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 30, 2021
@merlimat merlimat added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Nov 30, 2021
@merlimat merlimat added this to the 2.10.0 milestone Nov 30, 2021
michaeljmarshall
michaeljmarshall requested changes Dec 3, 2021
View reviewed changes
Copy link
Member

@michaeljmarshall michaeljmarshall left a comment

Choose a reason for hiding this comment

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

Thanks for you contribution @tomscut. I'd like to get some consensus that changing from nanoTime to currentTimeMillis is the right change before we merge this PR.

pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java Outdated Show resolved Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java Outdated Show resolved Hide resolved
@tomscut
Copy link
Contributor Author

tomscut commented Dec 15, 2021

Hi @merlimat @michaeljmarshall @hangc0276 , could you please help review again. Thanks a lot.

@hangc0276 hangc0276 merged commit 891660e into apache:master Dec 23, 2021
Technoboy- pushed a commit to Technoboy-/pulsar that referenced this pull request Dec 29, 2021
@tomscut @Technoboy-
[Producer] Change the time units from ns to ms (apache#13057)
d03ccd8 
### Motivation
The time unit in this exception message is ns, which is not very readable. We can change it from ns to ms.
```
org.apache.pulsar.client.api.PulsarClientException$TimeoutException:
The producer xxx can not send message to the topic xxx within given timeout : createdAt 461913074 ns ago, firstSentAt 29545553038276935 ns ago, lastSentAt 29545553038276935 ns ago, retryCount 0 at org.apache.pulsar.client.api.PulsarClientException.unwrap(PulsarClientException.java:916)
at org.apache.pulsar.client.impl.TypedMessageBuilderImpl.send(TypedMessageBuilderImpl.java:93)
at org.apache.pulsar.client.impl.ProducerBase.send(ProducerBase.java:63)
at com.yum.boh.oh.service.impl.StoreOrderPostServiceImpl.generalProcessing(StoreOrderPostServiceImpl.java:272)
at com.yum.boh.oh.service.impl.StoreOrderPostServiceImpl.saveThirdOrder(StoreOrderPostServiceImpl.java:72)
at com.yum.boh.oh.controller.StoreOrderController.postOrderInfo$original$T8425mfx(StoreOrderController.java:39)
at com.yum.boh.oh.controller.StoreOrderController.postOrderInfo$original$T8425mfx$accessor$vJljNzML(StoreOrderController.java)
at com.yum.boh.oh.controller.StoreOrderController$auxiliary$nysalhgy.call(Unknown Source)
at org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstMethodsInter.intercept(InstMethodsInter.java:86)
```

### Modifications
Change the time units from ns to ms for ProducerImpl#OpSendMsg.
codelipenghui pushed a commit that referenced this pull request Dec 30, 2021
@tomscut @codelipenghui
[Producer] Change the time units from ns to ms (#13057)
048154c 
### Motivation
The time unit in this exception message is ns, which is not very readable. We can change it from ns to ms.
```
org.apache.pulsar.client.api.PulsarClientException$TimeoutException:
The producer xxx can not send message to the topic xxx within given timeout : createdAt 461913074 ns ago, firstSentAt 29545553038276935 ns ago, lastSentAt 29545553038276935 ns ago, retryCount 0 at org.apache.pulsar.client.api.PulsarClientException.unwrap(PulsarClientException.java:916)
at org.apache.pulsar.client.impl.TypedMessageBuilderImpl.send(TypedMessageBuilderImpl.java:93)
at org.apache.pulsar.client.impl.ProducerBase.send(ProducerBase.java:63)
at com.yum.boh.oh.service.impl.StoreOrderPostServiceImpl.generalProcessing(StoreOrderPostServiceImpl.java:272)
at com.yum.boh.oh.service.impl.StoreOrderPostServiceImpl.saveThirdOrder(StoreOrderPostServiceImpl.java:72)
at com.yum.boh.oh.controller.StoreOrderController.postOrderInfo$original$T8425mfx(StoreOrderController.java:39)
at com.yum.boh.oh.controller.StoreOrderController.postOrderInfo$original$T8425mfx$accessor$vJljNzML(StoreOrderController.java)
at com.yum.boh.oh.controller.StoreOrderController$auxiliary$nysalhgy.call(Unknown Source)
at org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstMethodsInter.intercept(InstMethodsInter.java:86)
```

### Modifications
Change the time units from ns to ms for ProducerImpl#OpSendMsg.

(cherry picked from commit 891660e)
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Dec 30, 2021
codelipenghui pushed a commit that referenced this pull request Dec 30, 2021
@tomscut @codelipenghui
[Producer] Change the time units from ns to ms (#13057)
b763f83 
### Motivation
The time unit in this exception message is ns, which is not very readable. We can change it from ns to ms.
```
org.apache.pulsar.client.api.PulsarClientException$TimeoutException:
The producer xxx can not send message to the topic xxx within given timeout : createdAt 461913074 ns ago, firstSentAt 29545553038276935 ns ago, lastSentAt 29545553038276935 ns ago, retryCount 0 at org.apache.pulsar.client.api.PulsarClientException.unwrap(PulsarClientException.java:916)
at org.apache.pulsar.client.impl.TypedMessageBuilderImpl.send(TypedMessageBuilderImpl.java:93)
at org.apache.pulsar.client.impl.ProducerBase.send(ProducerBase.java:63)
at com.yum.boh.oh.service.impl.StoreOrderPostServiceImpl.generalProcessing(StoreOrderPostServiceImpl.java:272)
at com.yum.boh.oh.service.impl.StoreOrderPostServiceImpl.saveThirdOrder(StoreOrderPostServiceImpl.java:72)
at com.yum.boh.oh.controller.StoreOrderController.postOrderInfo$original$T8425mfx(StoreOrderController.java:39)
at com.yum.boh.oh.controller.StoreOrderController.postOrderInfo$original$T8425mfx$accessor$vJljNzML(StoreOrderController.java)
at com.yum.boh.oh.controller.StoreOrderController$auxiliary$nysalhgy.call(Unknown Source)
at org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstMethodsInter.intercept(InstMethodsInter.java:86)
```

### Modifications
Change the time units from ns to ms for ProducerImpl#OpSendMsg.

(cherry picked from commit 891660e)
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lhotari lhotari lhotari left review comments

@merlimat merlimat merlimat approved these changes

@hangc0276 hangc0276 hangc0276 approved these changes

@nicoloboschi nicoloboschi nicoloboschi approved these changes

@michaeljmarshall michaeljmarshall michaeljmarshall approved these changes

Assignees

@tomscut tomscut

Labels
area/client cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.3 release/2.9.2 type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Milestone
2.10.0
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@tomscut @merlimat @lhotari @hangc0276 @nicoloboschi @michaeljmarshall @codelipenghui @gaoran10