You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "leaves12138 (via GitHub)" <gi...@apache.org> on 2023/11/01 15:16:30 UTC
[PR] [core] [bug] fix InMemoryBuffer preempt loop [incubator-paimon]
leaves12138 opened a new pull request, #2239:
URL: https://github.com/apache/incubator-paimon/pull/2239
`InMemoryBuffer` put method may cause loop preempt memory, and finally it will preempt itself.
And flush should never ask for memory.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] [bug] fix InMemoryBuffer preempt loop [incubator-paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #2239:
URL: https://github.com/apache/incubator-paimon/pull/2239#discussion_r1379621951
##########
paimon-core/src/main/java/org/apache/paimon/disk/InMemoryBuffer.java:
##########
@@ -190,4 +200,33 @@ public BinaryRow next() throws IOException {
@Override
public void close() {}
}
+
+ // Use this to return an empty iterator, instead of use an interface (virtual function call will
+ // cause performance loss)
+ private static class EmptyInMemoryBufferIterator extends InMemoryBufferIterator {
+
+ private EmptyInMemoryBufferIterator() {
+ super(null, new InternalRowSerializer());
+ }
+
+ @Override
+ public boolean advanceNext() {
+ return false;
+ }
+
+ @Override
+ public BinaryRow getRow() {
+ return null;
Review Comment:
throw new UnsupportedException
##########
paimon-core/src/main/java/org/apache/paimon/disk/InMemoryBuffer.java:
##########
@@ -126,6 +133,9 @@ long getCurrentDataBufferOffset() {
}
int getNumRecordBuffers() {
+ if (currentDataBufferOffset < 0) {
Review Comment:
if (!isInitialized) {
return 0;
}
##########
paimon-core/src/main/java/org/apache/paimon/disk/InMemoryBuffer.java:
##########
@@ -68,7 +72,8 @@ private void tryInitialize() {
@Override
public void reset() {
if (this.isInitialized) {
- this.currentDataBufferOffset = 0;
+ // we need to set -1 here to avoid been preempt
+ this.currentDataBufferOffset = -1;
Review Comment:
I don't know why to set to -1.
I think we should modify `MemoryPoolFactory.preemptMemory`, if (memoryOccupancy > 0) assign to max.
##########
paimon-core/src/main/java/org/apache/paimon/disk/InMemoryBuffer.java:
##########
@@ -110,7 +115,9 @@ public long memoryOccupancy() {
@Override
public InMemoryBufferIterator newIterator() {
- tryInitialize();
+ if (!isInitialized) {
+ return EMPTY_ITERATOR;
Review Comment:
Add comment: to avoid request memory.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] [bug] fix InMemoryBuffer preempt loop [incubator-paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #2239:
URL: https://github.com/apache/incubator-paimon/pull/2239
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] [bug] fix InMemoryBuffer preempt loop [incubator-paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #2239:
URL: https://github.com/apache/incubator-paimon/pull/2239#discussion_r1379646587
##########
paimon-common/src/main/java/org/apache/paimon/data/SimpleCollectingOutputView.java:
##########
@@ -52,6 +52,7 @@ public SimpleCollectingOutputView(
this.fullSegments = fullSegmentTarget;
this.memorySource = memSource;
this.fullSegments.add(getCurrentSegment());
+ segmentNum = 0;
Review Comment:
revert this.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] [bug] fix InMemoryBuffer preempt loop [incubator-paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on code in PR #2239:
URL: https://github.com/apache/incubator-paimon/pull/2239#discussion_r1379691922
##########
paimon-core/src/test/java/org/apache/paimon/disk/InMemoryBufferTest.java:
##########
@@ -143,4 +147,70 @@ public void testEmpty() throws Exception {
RowBufferIterator iterator = buffer.newIterator();
assertThat(iterator.advanceNext()).isFalse();
}
+
+ @Test
+ public void testMemoryPoolWorksWellWithInMemoryBuffer() throws Exception {
Review Comment:
done
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] [bug] fix InMemoryBuffer preempt loop [incubator-paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #2239:
URL: https://github.com/apache/incubator-paimon/pull/2239#discussion_r1379687773
##########
paimon-core/src/test/java/org/apache/paimon/disk/InMemoryBufferTest.java:
##########
@@ -143,4 +147,70 @@ public void testEmpty() throws Exception {
RowBufferIterator iterator = buffer.newIterator();
assertThat(iterator.advanceNext()).isFalse();
}
+
+ @Test
+ public void testMemoryPoolWorksWellWithInMemoryBuffer() throws Exception {
Review Comment:
There are lots of code style issues, you can check your ide 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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org