You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "khanimteyaz (GitHub)" <gi...@apache.org> on 2018/12/28 16:08:26 UTC
[GitHub] [incubator-dubbo] khanimteyaz opened pull request #3090:
Acesslog dateformat enhancemnet
## What is the purpose of the change
AcceLogFilter message creation and last and current check was performing by each time simple date formate object creation. In this version of code I have use the ThreadLocal for each thread wise to create simple date format object instead of each call
https://github.com/apache/incubator-dubbo/issues/3026
In this PR
1. I have moved the access log creation and writing to file into a single thread and reusing a single instance of **SimpleDateFormat**
2. Refactored code to separate our and group related task in separate method and have enhance the readability by using
- Method naming
- Reducing big methods to small.
This PR is a resurrection of old PR
1. 3027 (https://github.com/apache/incubator-dubbo/pull/3027) PR
2. 3080 (https://github.com/apache/incubator-dubbo/pull/3080)
Sorry for the inconvenience
## Brief changelog
XXXXX
## Verifying this change
Runnig UT and verifying generated access log.
Follow this checklist to help us incorporate your contribution quickly and easily:
- [x] Make sure there is a [GITHUB_issue] (https://github.com/apache/incubator-dubbo/issues/3026) field for the change (usually before you start working on it). Trivial changes like typos do not require a GITHUB issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
- [ ] Format the pull request title like `[Dubbo-XXX] Fix UnknownException when host config not exist #XXX`. Each commit in the pull request should have a meaningful subject line and body.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in [test module](https://github.com/apache/incubator-dubbo/tree/master/dubbo-test).
- [ ] Run `mvn clean install -DskipTests=false` & `mvn clean test-compile failsafe:integration-test` to make sure unit-test and integration-test pass.
- [ ] If this contribution is large, please follow the [Software Donation Guide](https://github.com/apache/incubator-dubbo/wiki/Software-donation-guide).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=h1) Report
> Merging [#3090](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/09cbf8f616876b205b11aee942d90cc43f55c904?src=pr&el=desc) will **increase** coverage by `0.16%`.
> The diff coverage is `85.29%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #3090 +/- ##
============================================
+ Coverage 63.5% 63.67% +0.16%
Complexity 75 75
============================================
Files 653 653
Lines 28251 28238 -13
Branches 4816 4781 -35
============================================
+ Hits 17942 17980 +38
+ Misses 8040 8003 -37
+ Partials 2269 2255 -14
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...a/org/apache/dubbo/rpc/filter/AccessLogFilter.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9maWx0ZXIvQWNjZXNzTG9nRmlsdGVyLmphdmE=) | `83.78% <81.81%> (+12.67%)` | `0 <0> (ø)` | :arrow_down: |
| [...va/org/apache/dubbo/rpc/support/AccessLogData.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9zdXBwb3J0L0FjY2Vzc0xvZ0RhdGEuamF2YQ==) | `88.57% <88.57%> (ø)` | `0 <0> (?)` | |
| [.../remoting/transport/netty4/NettyClientHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHk0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvbmV0dHk0L05ldHR5Q2xpZW50SGFuZGxlci5qYXZh) | `75% <0%> (-11.12%)` | `0% <0%> (ø)` | |
| [...ng/zookeeper/zkclient/ZkclientZookeeperClient.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3Rpbmctem9va2VlcGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy96b29rZWVwZXIvemtjbGllbnQvWmtjbGllbnRab29rZWVwZXJDbGllbnQuamF2YQ==) | `55% <0%> (-7.75%)` | `0% <0%> (ø)` | |
| [...ava/org/apache/dubbo/rpc/cluster/Configurator.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvQ29uZmlndXJhdG9yLmphdmE=) | `92.3% <0%> (-7.7%)` | `0% <0%> (ø)` | |
| [...in/java/org/apache/dubbo/common/utils/JVMUtil.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vdXRpbHMvSlZNVXRpbC5qYXZh) | `73.58% <0%> (-7.55%)` | `0% <0%> (ø)` | |
| [...ava/org/apache/dubbo/config/DubboShutdownHook.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9EdWJib1NodXRkb3duSG9vay5qYXZh) | `78.12% <0%> (-6.25%)` | `0% <0%> (ø)` | |
| [...he/dubbo/remoting/transport/netty/NettyServer.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JlbW90aW5nL3RyYW5zcG9ydC9uZXR0eS9OZXR0eVNlcnZlci5qYXZh) | `67.85% <0%> (-3.58%)` | `0% <0%> (ø)` | |
| [.../src/main/java/org/apache/dubbo/rpc/RpcStatus.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9ScGNTdGF0dXMuamF2YQ==) | `68.13% <0%> (-2.2%)` | `0% <0%> (ø)` | |
| [...dubbo/remoting/exchange/support/DefaultFuture.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9leGNoYW5nZS9zdXBwb3J0L0RlZmF1bHRGdXR1cmUuamF2YQ==) | `73.15% <0%> (-2.02%)` | `0% <0%> (ø)` | |
| ... and [52 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=footer). Last update [09cbf8f...47ca7c4](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3090: Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
yes. I will remove it.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] jasonjoo2010 commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "jasonjoo2010 (GitHub)" <gi...@apache.org>.
> @khanimteyaz
> Would pls check whether 'FastDataFormat' is better?
> :)
>
> Thx for your suggestion. @jasonjoo2010
But it's in `commons-lang` or `commons-lang3` library maybe we need introduce a new library to project. It's the only point.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
@khanimteyaz
nice work!
I will take a look.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on pull request #3090:
Acesslog dateformat enhancemnet
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
105，seems like this line separate the doc and the code?
:)
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
@zonghaishang
Whole log parsing and writing is happening in a single thread environment , irrespective of invoke. Would you provide us more details so that I can fix it if I can ?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3090: Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Done.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] dubbo-bot commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "dubbo-bot (GitHub)" <gi...@apache.org>.
Ping @khanimteyaz . Conflict happens after merging a previous commit. Please rebase the branch against master and push it back again. Thanks a lot.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on pull request #3090:
Acesslog dateformat enhancemnet
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
emmm...
What is this factory produce? Maybe I haven't get your point~ Can u give more details?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=h1) Report
> Merging [#3090](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/09cbf8f616876b205b11aee942d90cc43f55c904?src=pr&el=desc) will **increase** coverage by `0.21%`.
> The diff coverage is `85.4%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #3090 +/- ##
============================================
+ Coverage 63.5% 63.72% +0.21%
Complexity 75 75
============================================
Files 653 654 +1
Lines 28251 28256 +5
Branches 4816 4781 -35
============================================
+ Hits 17942 18005 +63
+ Misses 8040 7997 -43
+ Partials 2269 2254 -15
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...n/java/org/apache/dubbo/common/utils/DateUtil.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vdXRpbHMvRGF0ZVV0aWwuamF2YQ==) | `100% <100%> (ø)` | `0 <0> (?)` | |
| [...a/org/apache/dubbo/rpc/filter/AccessLogFilter.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9maWx0ZXIvQWNjZXNzTG9nRmlsdGVyLmphdmE=) | `83.33% <80.95%> (+12.22%)` | `0 <0> (ø)` | :arrow_down: |
| [...va/org/apache/dubbo/rpc/support/AccessLogData.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9zdXBwb3J0L0FjY2Vzc0xvZ0RhdGEuamF2YQ==) | `88.73% <88.73%> (ø)` | `0 <0> (?)` | |
| [.../remoting/transport/netty4/NettyClientHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHk0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvbmV0dHk0L05ldHR5Q2xpZW50SGFuZGxlci5qYXZh) | `75% <0%> (-11.12%)` | `0% <0%> (ø)` | |
| [...bo/rpc/cluster/support/FailbackClusterInvoker.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvc3VwcG9ydC9GYWlsYmFja0NsdXN0ZXJJbnZva2VyLmphdmE=) | `67.21% <0%> (-8.2%)` | `0% <0%> (ø)` | |
| [...ng/zookeeper/zkclient/ZkclientZookeeperClient.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3Rpbmctem9va2VlcGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy96b29rZWVwZXIvemtjbGllbnQvWmtjbGllbnRab29rZWVwZXJDbGllbnQuamF2YQ==) | `55% <0%> (-7.75%)` | `0% <0%> (ø)` | |
| [...ava/org/apache/dubbo/rpc/cluster/Configurator.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvQ29uZmlndXJhdG9yLmphdmE=) | `92.3% <0%> (-7.7%)` | `0% <0%> (ø)` | |
| [...onfig/spring/extension/SpringExtensionFactory.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvZXh0ZW5zaW9uL1NwcmluZ0V4dGVuc2lvbkZhY3RvcnkuamF2YQ==) | `78.04% <0%> (-7.32%)` | `0% <0%> (ø)` | |
| [...apache/dubbo/remoting/transport/AbstractCodec.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvQWJzdHJhY3RDb2RlYy5qYXZh) | `76.92% <0%> (-3.85%)` | `0% <0%> (ø)` | |
| [...dubbo/remoting/exchange/support/DefaultFuture.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9leGNoYW5nZS9zdXBwb3J0L0RlZmF1bHRGdXR1cmUuamF2YQ==) | `71.81% <0%> (-3.36%)` | `0% <0%> (ø)` | |
| ... and [99 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=footer). Last update [09cbf8f...154b560](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] dubbo-bot commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "dubbo-bot (GitHub)" <gi...@apache.org>.
Ping @khanimteyaz . Conflict happens after merging a previous commit. Please rebase the branch against master and push it back again. Thanks a lot.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
@khanimteyaz
Would pls check whether 'FastDataFormat' is better?
:)
Thx for your suggestion. @jasonjoo2010
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
Do we copy code into dubbo?
Seems like we can depend on common-langs package.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] jasonjoo2010 commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "jasonjoo2010 (GitHub)" <gi...@apache.org>.
> @jasonjoo2010
> Is the class thread-safe?
Yes, it's thread-safe by implemented no sharing data and also including instances cache.
Generally we can just call `FastDataFormat.getInstance("yyyyMMdd")` as same as `new SimpleDateFormat("yyyyMMdd")`.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
@zonghaishang could you please provide the details about the thread safety so that I can work on this?
@beiwei30 Is there something to incorporate which you feel here?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
@zonghaishang would you care to provide some detail on the thread safety, so that I can incorporate them? FYI I have provided the comment in previous section about the implementation.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
@zonghaishang
Whole log parsing and writing is happening in a single thread environment , irrespective of invokes. Would you provide more details so that I can fix it if I can ?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] jasonjoo2010 commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "jasonjoo2010 (GitHub)" <gi...@apache.org>.
> Thanks @carryxyh @jasonjoo2010 @lixiaojiee for reviewing the PR and provding feedback. I have incorporated the changes and have included common-lang3 as well. Could you please have a look again.
It looks good now.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3090: Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
😄
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] dubbo-bot commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "dubbo-bot (GitHub)" <gi...@apache.org>.
Ping @khanimteyaz . Conflict happens after merging a previous commit. Please rebase the branch against master and push it back again. Thanks a lot.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz closed pull request #3090:
Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
[ pull request closed by khanimteyaz ]
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3090: Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
done
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] dubbo-bot commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "dubbo-bot (GitHub)" <gi...@apache.org>.
Ping @khanimteyaz . Conflict happens after merging a previous commit. Please rebase the branch against master and push it back again. Thanks a lot.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] jasonjoo2010 commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "jasonjoo2010 (GitHub)" <gi...@apache.org>.
> Do we copy code into dubbo?
> Seems like we can depend on common-langs package.
And more plz pay attention there are two different versions for commons-lang:
* commons-lang
* commons-lang3 (JRE >= 1.5)
If we can make it 1.5 and above `lang3` is recommended.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3090: Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Yes we make. The only reason I avoid it was, double type one for defining variable and second for value of the variable, e.g.
private String group="group"
where using enum, the enum filed it self serve the both.
I will modify it.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] jasonjoo2010 commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "jasonjoo2010 (GitHub)" <gi...@apache.org>.
Why not introducing `FastDateFormat`?
I think it's more simple and make sense.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3090: Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
it provide the storage object which its consumer object,
e.g.
```
Store store= InMemoryQueueStorageFactory.getInstance().getStore();
store.add(accessLog);
```
something like above. The above example what I hvae given may not be very intuitive itself for explaination, but for high level understanding it might help. If you want I might create a sample branch to demonestrate in more detail way by having some implementation. What do you say?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
I think the whole log parsing and writing is happening in a single thread environment , irrespective of invoke. Would you provide us more details so that I can fix it if I can ?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
I didn't look into the change closely, but it looks like we are now moving the date formatter into one single thread to avoid contention completely, which I believe it is the right direction as I pointed out in the old pull request :)
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] lixiaojiee commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "lixiaojiee (GitHub)" <gi...@apache.org>.
SimpleDateFormat is not thread-safe, which is why we have to create new objects every time before. I did some research. Java8 currently has a thread-safe date tool called LocalDateTime. After reviewing @jasonjoo2010 's recommendation, I did some testing and found that FastDateFormat performs much better than the other two. Therefore, I also recommend FastDateFormat, but I prefer to bring it into the dubbo-commom package as a common date tool for other modules to reuse.You can refer to the data in the table below. The first column is the number of samples, and the following data is the average time corresponding to each sample in ms.
| SimpleDateFormat | FastDateFormat | LocalDateTime
-- | -- | -- | --
10 | 3 | 1 | 77
100 | 10 | 4 | 83
1000 | 54 | 26 | 137
10000 | 170 | 52 | 168
100000 | 470 | 153 | 341
1000000 | 1827 | 501 | 1024
10000000 | 11591 | 3473 | 6228
**Note** : For environmental reasons, the above data are for reference only
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
I have updated with comment, would you review my pr?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
Sure
We only use 1.8 and above.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] dubbo-bot commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "dubbo-bot (GitHub)" <gi...@apache.org>.
Ping @khanimteyaz . Conflict happens after merging a previous commit. Please rebase the branch against master and push it back again. Thanks a lot.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] lixiaojiee commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "lixiaojiee (GitHub)" <gi...@apache.org>.
> Thanks @carryxyh @jasonjoo2010 @lixiaojiee for reviewing the PR and provding feedback. I have incorporated the changes and have included common-lang3 as well. Could you please have a look again.
lgtm
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on pull request #3090:
Acesslog dateformat enhancemnet
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
Why not make this to be fileds like:
```
private String group;
private String version;
...
```
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Thanks @carryxyh @jasonjoo2010 @lixiaojiee for reviewing the PR and provding feedback. I have incorporated the changes and have included common-lang3 as well. Could you please have a look again.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] dubbo-bot commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "dubbo-bot (GitHub)" <gi...@apache.org>.
Ping @khanimteyaz . Conflict happens after merging a previous commit. Please rebase the branch against master and push it back again. Thanks a lot.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on pull request #3090:
Acesslog dateformat enhancemnet
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
Can we use computeIfAbsent to optimize this?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on pull request #3090:
Acesslog dateformat enhancemnet
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
Can this field be static?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3090: Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
I think it make sense now to make it static as it is now bound to single thread.
In addition to this, I was thinking (correct me If I am wrong), instead of defining the logQueue as a member of this class, we can it get through a factory (although factory will supply single queue only), so that in future even if we have change it based on our defined we don't have to touch this class. May be we can do this as part of separate PR. What do you say?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3090: Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Done
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] zonghaishang commented on pull request
#3090: Acesslog dateformat enhancemnet
Posted by "zonghaishang (GitHub)" <gi...@apache.org>.
not thread safe
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3090: Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Done.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3090: Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
ok. got it.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=h1) Report
> Merging [#3090](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/09cbf8f616876b205b11aee942d90cc43f55c904?src=pr&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `86.01%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #3090 +/- ##
============================================
+ Coverage 63.5% 63.53% +0.02%
Complexity 75 75
============================================
Files 653 654 +1
Lines 28251 28312 +61
Branches 4816 4817 +1
============================================
+ Hits 17942 17987 +45
- Misses 8040 8055 +15
- Partials 2269 2270 +1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...a/org/apache/dubbo/rpc/filter/AccessLogFilter.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9maWx0ZXIvQWNjZXNzTG9nRmlsdGVyLmphdmE=) | `84.61% <82.85%> (+13.5%)` | `0 <0> (ø)` | :arrow_down: |
| [...va/org/apache/dubbo/rpc/support/AccessLogData.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9zdXBwb3J0L0FjY2Vzc0xvZ0RhdGEuamF2YQ==) | `89.04% <89.04%> (ø)` | `0 <0> (?)` | |
| [...onfig/spring/extension/SpringExtensionFactory.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvZXh0ZW5zaW9uL1NwcmluZ0V4dGVuc2lvbkZhY3RvcnkuamF2YQ==) | `78.04% <0%> (-7.32%)` | `0% <0%> (ø)` | |
| [...bbo/registry/support/ProviderConsumerRegTable.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS9zdXBwb3J0L1Byb3ZpZGVyQ29uc3VtZXJSZWdUYWJsZS5qYXZh) | `80.48% <0%> (-4.88%)` | `0% <0%> (ø)` | |
| [...he/dubbo/remoting/transport/netty/NettyServer.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JlbW90aW5nL3RyYW5zcG9ydC9uZXR0eS9OZXR0eVNlcnZlci5qYXZh) | `67.85% <0%> (-3.58%)` | `0% <0%> (ø)` | |
| [...org/apache/dubbo/rpc/protocol/AbstractInvoker.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9wcm90b2NvbC9BYnN0cmFjdEludm9rZXIuamF2YQ==) | `62.9% <0%> (-3.23%)` | `0% <0%> (ø)` | |
| [.../apache/dubbo/remoting/transport/AbstractPeer.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvQWJzdHJhY3RQZWVyLmphdmE=) | `58.69% <0%> (-2.18%)` | `0% <0%> (ø)` | |
| [...a/org/apache/dubbo/monitor/dubbo/DubboMonitor.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tbW9uaXRvci9kdWJiby1tb25pdG9yLWRlZmF1bHQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL21vbml0b3IvZHViYm8vRHViYm9Nb25pdG9yLmphdmE=) | `87.85% <0%> (-1.87%)` | `0% <0%> (ø)` | |
| [...he/dubbo/registry/multicast/MulticastRegistry.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktbXVsdGljYXN0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS9tdWx0aWNhc3QvTXVsdGljYXN0UmVnaXN0cnkuamF2YQ==) | `63.79% <0%> (-1.73%)` | `0% <0%> (ø)` | |
| [...rpc/protocol/dubbo/telnet/InvokeTelnetHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL3RlbG5ldC9JbnZva2VUZWxuZXRIYW5kbGVyLmphdmE=) | `72.5% <0%> (-1.67%)` | `0% <0%> (ø)` | |
| ... and [7 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=footer). Last update [09cbf8f...a3592ed](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on pull request #3090:
Acesslog dateformat enhancemnet
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
Seems like this filed is never used.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] lixiaojiee commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "lixiaojiee (GitHub)" <gi...@apache.org>.
SimpleDateFormat is not thread-safe, which is why we have to create new objects every time before. I did some research. Java8 currently has A thread-safe date tool called LocalDateTime. After reviewing @jasonjoo2010 's recommendation, I did some testing and found that FastDateFormat performs much better than the other two. Therefore, I also recommend FastDateFormat, but I prefer to bring it into the dubbo-commom package as a common date tool for other modules to reuse.You can refer to the data in the table below. The first column is the number of samples, and the following data is the average time corresponding to each sample in ms.
| SimpleDateFormat | FastDateFormat | LocalDateTime
-- | -- | -- | --
10 | 3 | 1 | 77
100 | 10 | 4 | 83
1000 | 54 | 26 | 137
10000 | 170 | 52 | 168
100000 | 470 | 153 | 341
1000000 | 1827 | 501 | 1024
10000000 | 11591 | 3473 | 6228
**Note** : For environmental reasons, the above data are for reference only
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=h1) Report
> Merging [#3090](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/09cbf8f616876b205b11aee942d90cc43f55c904?src=pr&el=desc) will **increase** coverage by `0.16%`.
> The diff coverage is `85.4%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #3090 +/- ##
============================================
+ Coverage 63.5% 63.67% +0.16%
Complexity 75 75
============================================
Files 653 653
Lines 28251 28239 -12
Branches 4816 4781 -35
============================================
+ Hits 17942 17980 +38
+ Misses 8040 8006 -34
+ Partials 2269 2253 -16
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...a/org/apache/dubbo/rpc/filter/AccessLogFilter.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9maWx0ZXIvQWNjZXNzTG9nRmlsdGVyLmphdmE=) | `84% <82.08%> (+12.88%)` | `0 <0> (ø)` | :arrow_down: |
| [...va/org/apache/dubbo/rpc/support/AccessLogData.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9zdXBwb3J0L0FjY2Vzc0xvZ0RhdGEuamF2YQ==) | `88.57% <88.57%> (ø)` | `0 <0> (?)` | |
| [...ng/zookeeper/zkclient/ZkclientZookeeperClient.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3Rpbmctem9va2VlcGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy96b29rZWVwZXIvemtjbGllbnQvWmtjbGllbnRab29rZWVwZXJDbGllbnQuamF2YQ==) | `55% <0%> (-7.75%)` | `0% <0%> (ø)` | |
| [...ava/org/apache/dubbo/rpc/cluster/Configurator.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvQ29uZmlndXJhdG9yLmphdmE=) | `92.3% <0%> (-7.7%)` | `0% <0%> (ø)` | |
| [...e/dubbo/remoting/transport/netty/NettyChannel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JlbW90aW5nL3RyYW5zcG9ydC9uZXR0eS9OZXR0eUNoYW5uZWwuamF2YQ==) | `57.64% <0%> (-4.71%)` | `0% <0%> (ø)` | |
| [...c/main/java/org/apache/dubbo/rpc/RpcException.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9ScGNFeGNlcHRpb24uamF2YQ==) | `82.75% <0%> (-3.45%)` | `0% <0%> (ø)` | |
| [.../src/main/java/org/apache/dubbo/rpc/RpcStatus.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9ScGNTdGF0dXMuamF2YQ==) | `68.13% <0%> (-2.2%)` | `0% <0%> (ø)` | |
| [.../apache/dubbo/remoting/transport/AbstractPeer.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvQWJzdHJhY3RQZWVyLmphdmE=) | `58.69% <0%> (-2.18%)` | `0% <0%> (ø)` | |
| [...he/dubbo/registry/multicast/MulticastRegistry.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktbXVsdGljYXN0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS9tdWx0aWNhc3QvTXVsdGljYXN0UmVnaXN0cnkuamF2YQ==) | `63.79% <0%> (-1.73%)` | `0% <0%> (ø)` | |
| [...rpc/protocol/dubbo/telnet/InvokeTelnetHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1kdWJiby9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL3Byb3RvY29sL2R1YmJvL3RlbG5ldC9JbnZva2VUZWxuZXRIYW5kbGVyLmphdmE=) | `72.5% <0%> (-1.67%)` | `0% <0%> (ø)` | |
| ... and [48 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3090/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=footer). Last update [09cbf8f...47ca7c4](https://codecov.io/gh/apache/incubator-dubbo/pull/3090?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
@carryxyh could you please review this.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3090: Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
I think, I did not understand this one. Which line you wanted me to remove?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on pull request #3090:
Acesslog dateformat enhancemnet
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
Personally think that it is not necessary, because it will make the code more complicated. I think that the current structure is relatively simple, haha.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
@zonghaishang
Hi, yiji, would u pls check this pr again?
@khanimteyaz
Let's wait for his feedback. Seems like yiji is very busy recently. :)
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
@carryxyh I have made the changes, would you plz review it.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on pull request #3090:
Acesslog dateformat enhancemnet
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
remove this line.
:)
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
@zonghaishang would it be possible for you to have a look.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] carryxyh commented on issue #3090:
Acesslog dateformat enhancemnet
Posted by "carryxyh (GitHub)" <gi...@apache.org>.
@jasonjoo2010
Is the class thread-safe?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3090 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org