-
Notifications
You must be signed in to change notification settings - Fork 12
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.
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
adds soft pool cleaner, configuration and allocation limiter for CollectColumnValuesJob #3618
base: develop
Are you sure you want to change the base?
Conversation
…ectColumnValuesJob
// We use a semaphore here to pace the allocation and sending of messages (the number of permits is magic) | ||
final Semaphore semaphore = new Semaphore(4); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@awildturtok kann ich auch gerne wieder raus nehmen. Habe es noch nicht mit großen Daten getestet
*/ | ||
@NotNull | ||
@Valid | ||
private DataSize messageChunkSize = DataSize.kilobytes(2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das war vorher auf 2 MB gepinnt, aber wir haben die selten ausgenutzt, ich will es jetzt nochmal mit kleineren Buffern testen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ja das war ja damals auf 32MB. Dann mach doch lieber 4KB, oder versuch direkt an die pagesize zu kommen.
private final Supplier<T> supplier; | ||
private final ClusterConfig config; | ||
private final Thread poolCleaner = new Thread(this::cleanPool, "SoftPool Cleaner"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich habe hier einen Cleaner eingeführt, da ich gemerkt habe, dass sich hier die Referenzen an sammeln und die IoBuffer sonst erst abgeräumt werden, wenn der RAM vollläuft. Genau dann werden sie aber auch normaler weise gebraucht
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bisschen ein mischmasch aus Konzepten. die poolSize scheint keine Verwendung zu haben.
Auch zu bedenken, dass du so ggf IoBuffer erzeugst und dann direkt wieder dropst?
Generell der Ansatz aber gut.
@@ -63,21 +67,47 @@ public void react(Worker context) throws Exception { | |||
|
|||
final AtomicInteger done = new AtomicInteger(); | |||
|
|||
// We use a semaphore here to pace the allocation and sending of messages (the number of permits is magic) | |||
final Semaphore semaphore = new Semaphore(4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warum möchtest du denn keinen RateLimiter verwenden?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So wie ich mir das vorstelle ist ein Ratelimiter von der Größe her unboundet also wenn es mal stockt und Nachrichten können gerade nicht rausgeschickt werden, würde der Ratelimiter nach einiger Zeit wieder Threads reinlassen die weiter Speicher allozieren und die Pressure erhöhen.
Andererseits, wenn wir viele Nachrichten haben, die schnell versendet werden können, dann bremst uns der Ratelimiter
*/ | ||
@NotNull | ||
@Valid | ||
private DataSize messageChunkSize = DataSize.kilobytes(2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ja das war ja damals auf 32MB. Dann mach doch lieber 4KB, oder versuch direkt an die pagesize zu kommen.
Uninterruptibles.sleepUninterruptibly(config.getSoftPoolCleanerPause().toSeconds(), TimeUnit.SECONDS); | ||
|
||
log.trace("Running pool cleaner"); | ||
while (poolSize.get() > config.getSoftPoolBaselineSize()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wo wird die poolSize gezählt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Im borrow wird decremented und im offer incremented. Ist in den Logs versteckt, das ist gerade noch schlecht :D
No description provided.