snleee commented on code in PR #10045:
URL: https://github.com/apache/pinot/pull/10045#discussion_r1061082927
##########
pinot-connectors/pinot-flink-connector/src/main/java/org/apache/pinot/connector/flink/sink/PinotSinkFunction.java:
##########
@@ -151,4 +148,18 @@ private void flush()
LOG.info("Pinot segment uploaded to {}", segmentURI);
});
}
+
+ @Override
+ public List<GenericRow> snapshotState(long checkpointId, long timestamp) {
Review Comment:
How often this would be called from the Flink side? Do we incrementally call
this periodically over small `_pendingRows`?
Unless this snapshotState() gets called very frequently (i.e. getting called
for each input row), I think that storing all rows in memory can consume the
memory pretty quickly and we can easily go OOM when there are many columns.
##########
pinot-connectors/pinot-flink-connector/src/main/java/org/apache/pinot/connector/flink/sink/PinotSinkFunction.java:
##########
@@ -117,31 +123,22 @@ public void close()
@Override
public void invoke(T value, Context context)
throws Exception {
- _segmentWriter.collect(_recordConverter.convertToRow(value));
+ GenericRow row = _recordConverter.convertToRow(value);
+ _pendingRows.add(row);
+ _segmentWriter.collect(row);
_segmentNumRecord++;
- if (_segmentNumRecord > _segmentFlushMaxNumRecords) {
+ if (_segmentNumRecord >= _segmentFlushMaxNumRecords) {
Review Comment:
good catch :)
##########
pinot-connectors/pinot-flink-connector/src/main/java/org/apache/pinot/connector/flink/sink/PinotSinkFunction.java:
##########
@@ -96,7 +102,7 @@ public void close()
throws Exception {
LOG.info("Closing Pinot Sink");
try {
- if (_segmentNumRecord > 0) {
+ if (!((StreamingRuntimeContext)
this.getRuntimeContext()).isCheckpointingEnabled()) {
Review Comment:
I think that we should still check with `_segmentNumRecord > 0` ?
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
