simplify the mina stack by cumulating IoBuffers until there are enough #3633

thoniTUB · 2024-12-04T11:39:12Z

Tests fehlen noch und eine Strategie, sollten wir doch mal das IoBuffer limit erreichen.

thoniTUB · 2024-12-04T12:02:10Z

Tests hängen noch

awildturtok · 2024-12-09T10:51:23Z

backend/src/main/java/com/bakdata/conquery/io/mina/JacksonProtocolDecoder.java

+			log.error("Failed to decode message: {}", debuggedMessage , e);
+		}
+		finally {
+			// Set back the old limit, as the in buffer might already have data for a new object


verstehe den kommentar nicht ganz aber es klingt so als würden failende buffer konstant weiter akkumuliert werden, was das return false aber ausschließt

Das ist für den Fall dass wir einen Buffer bekommen, der ein Object hat plus noch etwas mehr Daten von dem Nachfolgenden Objekt.

Wir merken uns das eingehende Limit damit am Ende an der stelle weiter akkumuliert werden kann

Wir setzten das Limit genau auf das Ende des eingehenden Objekts damit mit wird Jackson nicht einen zu großen InputStream geben. Jackson kann jetzt failen, am Wahrscheinlichsten ist hier, dass Serializierung und Deserialisierung nicht zusammen passen.

Danach steht der Buffer genau am Anfang des nächsten Objekts, genau da wo auch das Limit hinzeigt, dann setzten wir das Limit wieder auf den Anfangswert, damit an dessen stelle für das nächste Objekt weiter akkumuliert wird.

Ich werde zu Sicherheit auch die Position hier nochmal auf das zwischenzeitliche Limit setzten, fall das debug JSON nicht den gesamten Buffer gelesen hat.

Moment, Mina gibt uns buffer in die es potentiell weiter schreiben wird? Wowie. Ich hatte erwartet, dass wenn die sowas machen, darüber abstrahiert wird für uns

backend/src/main/java/com/bakdata/conquery/io/mina/JacksonProtocolEncoder.java

awildturtok · 2024-12-09T11:00:03Z

backend/src/main/java/com/bakdata/conquery/io/mina/NetworkSession.java

@@ -30,22 +28,13 @@ public NetworkSession(IoSession session, int maxQueueLength) {

 	@Override
 	public WriteFuture send(final NetworkMessage<?> message) {
-		try {


warum kann der Teil dadurch weg?

ist wieder da :)

awildturtok · 2024-12-09T11:02:13Z

...in/java/com/bakdata/conquery/models/messages/namespaces/specific/CollectColumnValuesJob.java

@@ -67,7 +68,8 @@ public void react(Worker context) throws Exception {
 		final Map<TableId, List<Bucket>> table2Buckets = context.getStorage().getAllBuckets()
 																.collect(Collectors.groupingBy(Bucket::getTable));

-		final ListeningExecutorService jobsExecutorService = MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(MAX_THREADS));
+		BasicThreadFactory threadFactory = (new BasicThreadFactory.Builder()).namingPattern(this.getClass().getSimpleName() + "-Worker-%d").build();
+		final ListeningExecutorService jobsExecutorService = MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(MAX_THREADS, threadFactory));


awildturtok · 2024-12-09T11:04:34Z

backend/src/test/java/com/bakdata/conquery/util/support/TestConquery.java

@@ -177,7 +177,7 @@ public void waitUntilWorkDone() {

 				if (Duration.ofNanos(System.nanoTime() - started).toSeconds() > 10) {
 					started = System.nanoTime();
-					log.warn("waiting for done work for a long time", new Exception());
+					log.warn("waiting for done work for a long time");


Warum löscht du hier die Exception? Das ist dafür da um zu sehen wo lange gewartet wird

…o feature/simplify-mina-stack

awildturtok · 2024-12-16T08:56:32Z

backend/src/main/java/com/bakdata/conquery/io/mina/ChunkingFilter.java

+		log.trace("Sending {}. chunk: {} byte", chunkCount, ioBuffer.remaining());
+		DefaultWriteFuture future = new DefaultWriteFuture(session);
+
+		IoFutureListener<IoFuture> listener = handleWrittenChunk(writeRequest, writtenChunks, totalChunks);


Löst das deferren in den listener das deadlock von dem du gesprochen hattest?

Also finde es etwas mysteriös, dass der filter selber die nachricht verarbeitet sie aber noch nicht verschickt ist. Hängt wohl am Model von mina.

So ist die FilterChain implementiert, du gibst die einfach immer weiter die Chain herunter. Ist aber bei Jetty so ähnlich.

Den Deadlock hatte ich als ich im CollectColumnValuesJob auf die WriteFuture gewartet hatte. Aber erklären konnte ich es mir auch nicht.

awildturtok · 2024-12-16T09:53:49Z

backend/src/main/java/com/bakdata/conquery/io/mina/JacksonProtocolDecoder.java

+			log.error("Failed to decode message: {}", debuggedMessage , e);
+		}
+		finally {
+			// Set back the old limit, as the in buffer might already have data for a new object


Moment, Mina gibt uns buffer in die es potentiell weiter schreiben wird? Wowie. Ich hatte erwartet, dass wenn die sowas machen, darüber abstrahiert wird für uns

awildturtok · 2024-12-16T09:56:07Z

backend/src/main/java/com/bakdata/conquery/io/mina/JacksonProtocolEncoder.java

+		buf.setAutoExpand(true);
+
+
+		int oldPos = buf.position();


das ist doch nach definiont immer 0 oder?

ja so lange wir keine CachedBuffer nehmen (wo von Mina auch abrät ab einer bestimmten Java Version)

backend/src/main/java/com/bakdata/conquery/io/mina/JacksonProtocolEncoder.java

awildturtok · 2024-12-16T10:36:36Z

backend/src/main/java/com/bakdata/conquery/mode/cluster/ClusterImportHandler.java

+					log.trace("Sent Bucket {}", bucketId);
+					return;
+				}
+				log.warn("Failed to send Bucket {}", bucketId);


gibt es da einen cause?

backend/src/main/java/com/bakdata/conquery/models/config/ClusterConfig.java

backend/src/test/java/com/bakdata/conquery/io/mina/MinaStackTest.java

awildturtok · 2024-12-16T10:39:32Z

backend/src/test/java/com/bakdata/conquery/util/support/TestConquery.java

@@ -176,12 +176,12 @@ public void waitUntilWorkDone() {

 				if (Duration.ofNanos(System.nanoTime() - started).toSeconds() > 10) {
 					started = System.nanoTime();
-					log.warn("waiting for done work for a long time", new Exception());
+					log.warn("Waiting for done work for a long time", new Exception("This Exception marks the stacktrace, to show where we are waiting."));


simplify the mina stack by cumulating IoBuffers until there are enough

54dd9a7

thoniTUB requested a review from awildturtok December 4, 2024 11:39

Merge branch 'develop' into feature/simplify-mina-stack

2c73caf

thoniTUB added 2 commits December 4, 2024 15:29

prepare for test

f952c55

adds test for mina stack

633a13a

thoniTUB marked this pull request as ready for review December 4, 2024 17:21

thoniTUB self-assigned this Dec 4, 2024

thoniTUB added bug backend labels Dec 4, 2024

thoniTUB added 5 commits December 5, 2024 09:03

correct comment

90766cb

improve logging

ba93d3a

remove write queue

2a42812

flush explicitly

1cb6704

do not flush do not awaituninterruptibly

30175bf

awildturtok approved these changes Dec 9, 2024

View reviewed changes

thoniTUB added 12 commits December 9, 2024 15:57

adds chunking filter if send buffer size is set

ece3909

don'Ãt allocate new buffers in chunking filter

7a80eba

improve log messages

9b2602c

adds mixin to fix configs serdes

c29030a

add logging and stopwatches

fbcf8d6

Don't wait in the chain for chunks

6fb72a0

throttle cqpp import a bit to prevent excessive memory consuption

3f81ede

use bocking queue again to throttle iobuffer allocation

7cae1ef

by default set position in finally block to recover from unreadable data

fed018c

improve log message

a49b014

Merge branch 'develop' into feature/simplify-mina-stack

8eaa24e

Merge remote-tracking branch 'origin/feature/simplify-mina-stack' int…

1ef6569

…o feature/simplify-mina-stack

thoniTUB enabled auto-merge December 13, 2024 10:08

thoniTUB merged commit ef752fc into develop Dec 13, 2024
6 checks passed

awildturtok reviewed Dec 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

simplify the mina stack by cumulating IoBuffers until there are enough #3633

simplify the mina stack by cumulating IoBuffers until there are enough #3633

thoniTUB commented Dec 4, 2024

thoniTUB commented Dec 4, 2024

awildturtok Dec 9, 2024

thoniTUB Dec 13, 2024

awildturtok Dec 16, 2024

awildturtok Dec 9, 2024

thoniTUB Dec 13, 2024

awildturtok Dec 9, 2024

awildturtok Dec 9, 2024

awildturtok Dec 16, 2024

awildturtok Dec 16, 2024

thoniTUB Dec 17, 2024

awildturtok Dec 16, 2024

awildturtok Dec 16, 2024

thoniTUB Dec 17, 2024

awildturtok Dec 16, 2024

awildturtok Dec 16, 2024

simplify the mina stack by cumulating IoBuffers until there are enough #3633

simplify the mina stack by cumulating IoBuffers until there are enough #3633

Conversation

thoniTUB commented Dec 4, 2024

thoniTUB commented Dec 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment