You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/12/16 18:49:22 UTC

[GitHub] [nifi] mattyb149 opened a new pull request, #6791: NIFI-6501: Refactor CaptureChangeMySQL to not use an unbounded event queue

mattyb149 opened a new pull request, #6791:
URL: https://github.com/apache/nifi/pull/6791

   # Summary
   
   [NIFI-6501](https://issues.apache.org/jira/browse/NIFI-6501)
   
   # Tracking
   
   Please complete the following tracking steps prior to pull request creation.
   
   ### Issue Tracking
   
   - [x] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
   
   ### Pull Request Tracking
   
   - [x] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
   - [x] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
   
   ### Pull Request Formatting
   
   - [x] Pull Request based on current revision of the `main` branch
   - [x] Pull Request refers to a feature branch with one commit containing changes
   
   # Verification
   
   Please indicate the verification steps performed prior to pull request creation.
   
   ### Build
   
   - [ ] Build completed using `mvn clean install -P contrib-check`
     - [ ] JDK 8
     - [x] JDK 11
     - [ ] JDK 17
   
   ### Licensing
   
   - [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
   - [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
   
   ### Documentation
   
   - [ ] Documentation formatting appears as expected in rendered files
   


-- 
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: issues-unsubscribe@nifi.apache.org

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


[GitHub] [nifi] mattyb149 commented on a diff in pull request #6791: NIFI-6501: Refactor CaptureChangeMySQL to not use an unbounded event queue

Posted by GitBox <gi...@apache.org>.
mattyb149 commented on code in PR #6791:
URL: https://github.com/apache/nifi/pull/6791#discussion_r1055910980


##########
nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java:
##########
@@ -433,7 +434,7 @@ public class CaptureChangeMySQL extends AbstractSessionFactoryProcessor {
     private BinlogLifecycleListener lifecycleListener;
     private GtidSet gtidSet;
 
-    private final LinkedBlockingQueue<RawBinlogEvent> queue = new LinkedBlockingQueue<>();
+    private final BlockingQueue<RawBinlogEvent> queue = new LinkedBlockingQueue<>(100);

Review Comment:
   Since we're blocking waiting on the event to be added to the queue, @markap14 suggested we wouldn't need a very large buffer at all. I'm thinking maybe a larger buffer size would be for times when the processor is scheduled for a longer period than it should be? I welcome your and Mark's comments here, just went with his suggestion :)



-- 
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: issues-unsubscribe@nifi.apache.org

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


[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6791: NIFI-6501: Refactor CaptureChangeMySQL to not use an unbounded event queue

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6791:
URL: https://github.com/apache/nifi/pull/6791#discussion_r1052607445


##########
nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java:
##########
@@ -433,7 +434,7 @@ public class CaptureChangeMySQL extends AbstractSessionFactoryProcessor {
     private BinlogLifecycleListener lifecycleListener;
     private GtidSet gtidSet;
 
-    private final LinkedBlockingQueue<RawBinlogEvent> queue = new LinkedBlockingQueue<>();
+    private final BlockingQueue<RawBinlogEvent> queue = new LinkedBlockingQueue<>(100);

Review Comment:
   Is there a particular reason for selecting `100` as the queue size? Initial sets of change events could number in the thousands, but on the other hand, keeping this low avoids memory consumption issues.



-- 
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: issues-unsubscribe@nifi.apache.org

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


[GitHub] [nifi] mattyb149 commented on a diff in pull request #6791: NIFI-6501: Refactor CaptureChangeMySQL to not use an unbounded event queue

Posted by GitBox <gi...@apache.org>.
mattyb149 commented on code in PR #6791:
URL: https://github.com/apache/nifi/pull/6791#discussion_r1072612630


##########
nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java:
##########
@@ -433,7 +434,7 @@ public class CaptureChangeMySQL extends AbstractSessionFactoryProcessor {
     private BinlogLifecycleListener lifecycleListener;
     private GtidSet gtidSet;
 
-    private final LinkedBlockingQueue<RawBinlogEvent> queue = new LinkedBlockingQueue<>();
+    private final BlockingQueue<RawBinlogEvent> queue = new LinkedBlockingQueue<>(100);

Review Comment:
   I'll make it `1000` to help handle bursts of events,  but since it's now a true blocking queue, we shouldn't miss events and the processor should be draining the buffer as fast as the library is populating it.



-- 
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: issues-unsubscribe@nifi.apache.org

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


[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6791: NIFI-6501: Refactor CaptureChangeMySQL to not use an unbounded event queue

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6791:
URL: https://github.com/apache/nifi/pull/6791#discussion_r1055922009


##########
nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java:
##########
@@ -433,7 +434,7 @@ public class CaptureChangeMySQL extends AbstractSessionFactoryProcessor {
     private BinlogLifecycleListener lifecycleListener;
     private GtidSet gtidSet;
 
-    private final LinkedBlockingQueue<RawBinlogEvent> queue = new LinkedBlockingQueue<>();
+    private final BlockingQueue<RawBinlogEvent> queue = new LinkedBlockingQueue<>(100);

Review Comment:
   A large number of events can build up depending on the initial starting position in the binary log, or lots of activity. The value of `100` may still be best to avoid memory consumption, but I would have to take a closer look at the polling behavior to see whether there is value in a larger buffer to handle this kind of scenario.



-- 
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: issues-unsubscribe@nifi.apache.org

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


[GitHub] [nifi] exceptionfactory closed pull request #6791: NIFI-6501: Refactor CaptureChangeMySQL to not use an unbounded event queue

Posted by GitBox <gi...@apache.org>.
exceptionfactory closed pull request #6791: NIFI-6501: Refactor CaptureChangeMySQL to not use an unbounded event queue
URL: https://github.com/apache/nifi/pull/6791


-- 
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: issues-unsubscribe@nifi.apache.org

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