You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "Roiocam (via GitHub)" <gi...@apache.org> on 2024/04/12 02:16:36 UTC

[PR] use new Source.queue to avoid memory leak [pekko-persistence-jdbc]

Roiocam opened a new pull request, #175:
URL: https://github.com/apache/pekko-persistence-jdbc/pull/175

   (no comment)


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] use new Source.queue to avoid memory leak [pekko-persistence-jdbc]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin merged PR #175:
URL: https://github.com/apache/pekko-persistence-jdbc/pull/175


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] use new Source.queue to avoid memory leak [pekko-persistence-jdbc]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #175:
URL: https://github.com/apache/pekko-persistence-jdbc/pull/175#issuecomment-2051026371

   @Roiocam yes, please squash merge and cherry pick to 1.0.x 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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] use new Source.queue to avoid memory leak [pekko-persistence-jdbc]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #175:
URL: https://github.com/apache/pekko-persistence-jdbc/pull/175#discussion_r1561973148


##########
core/src/main/scala/org/apache/pekko/persistence/jdbc/journal/dao/BaseDao.scala:
##########
@@ -29,8 +29,8 @@ abstract class BaseDao[T] {
 
   def baseDaoConfig: BaseDaoConfig
 
-  val writeQueue: SourceQueueWithComplete[(Promise[Unit], Seq[T])] = Source
-    .queue[(Promise[Unit], Seq[T])](baseDaoConfig.bufferSize, OverflowStrategy.dropNew)
+  val writeQueue: BoundedSourceQueue[(Promise[Unit], Seq[T])] = Source

Review Comment:
   make it private now?



-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] use new Source.queue to avoid memory leak [pekko-persistence-jdbc]

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #175:
URL: https://github.com/apache/pekko-persistence-jdbc/pull/175#issuecomment-2053583335

   https://github.com/search?q=org%3Aapache%20SourceQueueWithComplete&type=code -- there is other Pekko code that uses SourceQueueWithComplete. Should that be rewritten too? Even if most of it is in tests, those tests can be viewed as a form of documentation of the code.


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] use new Source.queue to avoid memory leak [pekko-persistence-jdbc]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #175:
URL: https://github.com/apache/pekko-persistence-jdbc/pull/175#issuecomment-2053584720

   If it's been used with `dropNew` strategy ,then it can be updated with the new implementation by @jrudolph 


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] use new Source.queue to avoid memory leak [pekko-persistence-jdbc]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #175:
URL: https://github.com/apache/pekko-persistence-jdbc/pull/175#issuecomment-2050903345

   @Roiocam I was sending the same pr to https://github.com/SwissBorg/pekko-persistence-postgres/pull/32
   
   I think we should check other pekko persistence plugins too.


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] use new Source.queue to avoid memory leak [pekko-persistence-jdbc]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #175:
URL: https://github.com/apache/pekko-persistence-jdbc/pull/175#issuecomment-2054038580

   > > https://github.com/search?q=org%3Aapache%20SourceQueueWithComplete&type=code -- there is other Pekko code that uses SourceQueueWithComplete. Should that be rewritten too? Even if most of it is in tests, those tests can be viewed as a form of documentation of how to use the code.
   > 
   > like He-pin says, only `dropNew` has this issue, but I will check it when I am free.
   
   I checked that already,  it was backpresure strategy in other cases, which the current new source.queue doesn't support.


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] use new Source.queue to avoid memory leak [pekko-persistence-jdbc]

Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on PR #175:
URL: https://github.com/apache/pekko-persistence-jdbc/pull/175#issuecomment-2051023866

   > @Roiocam I was sending the same pr to [SwissBorg/pekko-persistence-postgres#32](https://github.com/SwissBorg/pekko-persistence-postgres/pull/32)
   > 
   > I think we should check other pekko persistence plugins too.
   
   I just checked and it seems that other plugins do not use queues for batching operations at writing events.


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] use new Source.queue to avoid memory leak [pekko-persistence-jdbc]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #175:
URL: https://github.com/apache/pekko-persistence-jdbc/pull/175#issuecomment-2053522794

   Oops, it should be merged with squash...


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org


Re: [PR] use new Source.queue to avoid memory leak [pekko-persistence-jdbc]

Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on PR #175:
URL: https://github.com/apache/pekko-persistence-jdbc/pull/175#issuecomment-2053950869

   > https://github.com/search?q=org%3Aapache%20SourceQueueWithComplete&type=code -- there is other Pekko code that uses SourceQueueWithComplete. Should that be rewritten too? Even if most of it is in tests, those tests can be viewed as a form of documentation of how to use the code.
   
   like He-pin says, only `dropNew` has this issue, but I will check it when I am free.


-- 
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: notifications-unsubscribe@pekko.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org