You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/09/21 04:18:54 UTC

[GitHub] [pulsar] coderzc opened a new pull request, #17756: [feat][broker][PIP-195] Implement BucketDelayedDeliveryTrackerFactory and load BucketDelayedDeliveryTracker - part5

coderzc opened a new pull request, #17756:
URL: https://github.com/apache/pulsar/pull/17756

   Master Issue: #16763 
   
   ### Motivation
   
   #16763 
   
   ### Modifications
   
   Implement BucketDelayedDeliveryTrackerFactory and load BucketDelayedDeliveryTracker
   
   ### Verifying this change
   
   - [x] Make sure that the change passes the CI checks.
   
   *(Please pick either of the following options)*
   
   This change is a trivial rework / code cleanup without any test coverage.
   
   *(or)*
   
   This change is already covered by existing tests, such as *(please describe tests)*.
   
   *(or)*
   
   This change added tests and can be verified as follows:
   
   *(example:)*
     - *Added integration tests for end-to-end deployment with large payloads (10MB)*
     - *Extended integration test for recovery after broker failure*
   
   ### Documentation
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ] `doc-required` 
   (Your PR needs to update docs and you will update later)
   
   - [x] `doc-not-needed` 
   (Please explain why)
   
   - [ ] `doc` 
   (Your PR contains doc changes)
   
   - [ ] `doc-complete`
   (Docs have been already added)
   
   ### Matching PR in forked repository
   
   PR in forked repository: xxx
   


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] codecov-commenter commented on pull request #17756: [feat][broker][PIP-195] Implement BucketDelayedDeliveryTrackerFactory and load BucketDelayedDeliveryTracker - part6

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #17756:
URL: https://github.com/apache/pulsar/pull/17756#issuecomment-1371761914

   # [Codecov](https://codecov.io/gh/apache/pulsar/pull/17756?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17756](https://codecov.io/gh/apache/pulsar/pull/17756?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6776c49) into [master](https://codecov.io/gh/apache/pulsar/commit/9ec1d071c7188a2db694e9d7b359faaf33cb076e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9ec1d07) will **decrease** coverage by `10.53%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pulsar/pull/17756/graphs/tree.svg?width=650&height=150&src=pr&token=acYqCpsK9J&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pulsar/pull/17756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master   #17756       +/-   ##
   =============================================
   - Coverage     47.73%   37.19%   -10.54%     
   + Complexity    10819     1987     -8832     
   =============================================
     Files           712      209      -503     
     Lines         69645    14452    -55193     
     Branches       7481     1577     -5904     
   =============================================
   - Hits          33242     5375    -27867     
   + Misses        32699     8493    -24206     
   + Partials       3704      584     -3120     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests | `37.19% <ø> (-10.54%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pulsar/pull/17756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...g/apache/pulsar/client/impl/ConnectionHandler.java](https://codecov.io/gh/apache/pulsar/pull/17756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL0Nvbm5lY3Rpb25IYW5kbGVyLmphdmE=) | `50.00% <0.00%> (-5.32%)` | :arrow_down: |
   | [...va/org/apache/pulsar/client/impl/ProducerBase.java](https://codecov.io/gh/apache/pulsar/pull/17756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1Byb2R1Y2VyQmFzZS5qYXZh) | `32.69% <0.00%> (-1.93%)` | :arrow_down: |
   | [...che/bookkeeper/mledger/impl/ManagedLedgerImpl.java](https://codecov.io/gh/apache/pulsar/pull/17756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9pbXBsL01hbmFnZWRMZWRnZXJJbXBsLmphdmE=) | | |
   | [.../org/apache/pulsar/broker/admin/AdminResource.java](https://codecov.io/gh/apache/pulsar/pull/17756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi9BZG1pblJlc291cmNlLmphdmE=) | | |
   | [.../pulsar/broker/admin/impl/SchemasResourceBase.java](https://codecov.io/gh/apache/pulsar/pull/17756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi9pbXBsL1NjaGVtYXNSZXNvdXJjZUJhc2UuamF2YQ==) | | |
   | [...apache/pulsar/broker/admin/v1/SchemasResource.java](https://codecov.io/gh/apache/pulsar/pull/17756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92MS9TY2hlbWFzUmVzb3VyY2UuamF2YQ==) | | |
   | [...apache/pulsar/broker/admin/v2/SchemasResource.java](https://codecov.io/gh/apache/pulsar/pull/17756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92Mi9TY2hlbWFzUmVzb3VyY2UuamF2YQ==) | | |
   | [...r/broker/delayed/DelayedDeliveryTrackerLoader.java](https://codecov.io/gh/apache/pulsar/pull/17756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9kZWxheWVkL0RlbGF5ZWREZWxpdmVyeVRyYWNrZXJMb2FkZXIuamF2YQ==) | | |
   | [...delayed/InMemoryDelayedDeliveryTrackerFactory.java](https://codecov.io/gh/apache/pulsar/pull/17756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9kZWxheWVkL0luTWVtb3J5RGVsYXllZERlbGl2ZXJ5VHJhY2tlckZhY3RvcnkuamF2YQ==) | | |
   | [...rg/apache/pulsar/broker/service/BrokerService.java](https://codecov.io/gh/apache/pulsar/pull/17756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL0Jyb2tlclNlcnZpY2UuamF2YQ==) | | |
   | ... and [495 more](https://codecov.io/gh/apache/pulsar/pull/17756?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] lhotari commented on pull request #17756: [feat][broker][PIP-195] Implement BucketDelayedDeliveryTrackerFactory and load BucketDelayedDeliveryTracker - part6

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #17756:
URL: https://github.com/apache/pulsar/pull/17756#issuecomment-1480663692

   This PR introduced a flaky test, #19902 . @coderzc Do you have a chance to fix it? Thanks


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] coderzc commented on pull request #17756: [feat][broker][PIP-195] Implement BucketDelayedDeliveryTrackerFactory and load BucketDelayedDeliveryTracker - part5

Posted by GitBox <gi...@apache.org>.
coderzc commented on PR #17756:
URL: https://github.com/apache/pulsar/pull/17756#issuecomment-1253186959

   Depend on #17611 and #17677


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] coderzc commented on a diff in pull request #17756: [feat][broker][PIP-195] Implement BucketDelayedDeliveryTrackerFactory and load BucketDelayedDeliveryTracker - part6

Posted by GitBox <gi...@apache.org>.
coderzc commented on code in PR #17756:
URL: https://github.com/apache/pulsar/pull/17756#discussion_r1060681729


##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/BucketDelayedDeliveryTest.java:
##########
@@ -0,0 +1,39 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.broker.service.persistent;
+
+import org.apache.pulsar.broker.delayed.BucketDelayedDeliveryTrackerFactory;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+
+public class BucketDelayedDeliveryTest extends DelayedDeliveryTest {

Review Comment:
   > And if we don't have any test method for this class. Will the before-class and after-class be executed?
   
   Yes, they can be executed.



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] codelipenghui merged pull request #17756: [feat][broker][PIP-195] Implement BucketDelayedDeliveryTrackerFactory and load BucketDelayedDeliveryTracker - part6

Posted by GitBox <gi...@apache.org>.
codelipenghui merged PR #17756:
URL: https://github.com/apache/pulsar/pull/17756


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] github-actions[bot] commented on pull request #17756: [feat][broker][PIP-195] Implement BucketDelayedDeliveryTrackerFactory and load BucketDelayedDeliveryTracker - part5

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #17756:
URL: https://github.com/apache/pulsar/pull/17756#issuecomment-1287583028

   The pr had no activity for 30 days, mark with Stale label.


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] codelipenghui commented on a diff in pull request #17756: [feat][broker][PIP-195] Implement BucketDelayedDeliveryTrackerFactory and load BucketDelayedDeliveryTracker - part6

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on code in PR #17756:
URL: https://github.com/apache/pulsar/pull/17756#discussion_r1059223987


##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/BucketDelayedDeliveryTest.java:
##########
@@ -0,0 +1,39 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.broker.service.persistent;
+
+import org.apache.pulsar.broker.delayed.BucketDelayedDeliveryTrackerFactory;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+
+public class BucketDelayedDeliveryTest extends DelayedDeliveryTest {

Review Comment:
   Please add the test group.
   
   And if we don't have any test method for this class. Will the before-class and after-class be executed?



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] codelipenghui closed pull request #17756: [feat][broker][PIP-195] Implement BucketDelayedDeliveryTrackerFactory and load BucketDelayedDeliveryTracker - part6

Posted by GitBox <gi...@apache.org>.
codelipenghui closed pull request #17756: [feat][broker][PIP-195] Implement BucketDelayedDeliveryTrackerFactory and load BucketDelayedDeliveryTracker - part6
URL: https://github.com/apache/pulsar/pull/17756


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] coderzc commented on a diff in pull request #17756: [feat][broker][PIP-195] Implement BucketDelayedDeliveryTrackerFactory and load BucketDelayedDeliveryTracker - part6

Posted by GitBox <gi...@apache.org>.
coderzc commented on code in PR #17756:
URL: https://github.com/apache/pulsar/pull/17756#discussion_r1060681729


##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/BucketDelayedDeliveryTest.java:
##########
@@ -0,0 +1,39 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.broker.service.persistent;
+
+import org.apache.pulsar.broker.delayed.BucketDelayedDeliveryTrackerFactory;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+
+public class BucketDelayedDeliveryTest extends DelayedDeliveryTest {

Review Comment:
   > And if we don't have any test method for this class. Will the before-class and after-class be executed?
   
   Yes, them can be executed.



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] coderzc commented on pull request #17756: [feat][broker][PIP-195] Implement BucketDelayedDeliveryTrackerFactory and load BucketDelayedDeliveryTracker - part6

Posted by GitBox <gi...@apache.org>.
coderzc commented on PR #17756:
URL: https://github.com/apache/pulsar/pull/17756#issuecomment-1369886643

   Wait for #19124 merged.


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org