DBZ-7845 [FEAT] adds reduced buffer implementation and test cases #75
Conversation
gaurav7261
commented
May 4, 2024
•
gaurav7261
commented
- adds reduced buffer
- created interface Buffer
- test cases for reduced buffer
- optimisation in buffer by avoiding copying in temporary buffer
|
Welcome as a new contributor to Debezium, @gaurav7261. Reviewers, please add missing author name(s) and alias name(s) to the COPYRIGHT.txt and Aliases.txt respectively. |
|
Hi @gaurav7261, thanks for your contribution. Please prefix the commit message(s) with the DBZ-xxx JIRA issue key. |
gaurav7261
changed the title
|
seems like checkstyle issue, looking into it, team any handy command that i can run to fix checkstyle, will be helpful => FIXED |
|
Hi @gaurav7261, thanks for your contribution. Please prefix the commit message(s) with the DBZ-xxx JIRA issue key. |
There was a problem hiding this comment.
Hi @gaurav7261, this looks good. I left a few inline suggestions and comments if you could take a look. Thanks!
src/main/java/io/debezium/connector/jdbc/JdbcChangeEventSink.java
Outdated
Show resolved
Hide resolved
src/main/java/io/debezium/connector/jdbc/JdbcChangeEventSink.java
Outdated
Show resolved
Hide resolved
src/main/java/io/debezium/connector/jdbc/JdbcSinkConnectorConfig.java
Outdated
Show resolved
Hide resolved
src/main/java/io/debezium/connector/jdbc/JdbcSinkConnectorConfig.java
Outdated
Show resolved
Hide resolved
src/main/java/io/debezium/connector/jdbc/dialect/postgres/EnumType.java
Outdated
Show resolved
Hide resolved
|
thanks for typo @Naros , i was pushing same |
There was a problem hiding this comment.
@gaurav7261 Thanks for the contribution! I left two comments related to two areas I am not sure I fully solved.
|
|
||
| @Override | ||
| public List<SinkRecordDescriptor> add(SinkRecordDescriptor recordDescriptor) { | ||
| ArrayList<SinkRecordDescriptor> flushed = new ArrayList<>(); |
There was a problem hiding this comment.
I see two issues here
- The type should be
List - Is there a way how to change the code so we can avoid copying the content of the buffer into the temporary variable? Ideally the result of method
flush()should be returned directly. BTW I don't have problem if buffer size limit is temporarily broken in case of PK struct change to simplify that perf improvement
There was a problem hiding this comment.
got it, working on it @jpechane
There was a problem hiding this comment.
also @jpechane , the initial logic copying the content of the buffer into the temporary variable, but this is soft copy , keeping pointers only for SinkRecordDescriptor object , not exact SinkRecordDescriptor object copy, correct ? I'm not sure, how much perf improvement will be there.
There was a problem hiding this comment.
I know it is a soft copy but let's suppose that you have very large buffer set, then even soft copies can add over the time.
There was a problem hiding this comment.
Also please bear in mind that we don't require single return from method so the code might at the end be easier to implement/comprehend if multiple returns are used.
There was a problem hiding this comment.
I have made the changes using variable isSchemaChanged, is it correct @jpechane ?
|
Hi @gaurav7261, thanks for your contribution. Please prefix the commit message(s) with the DBZ-xxx JIRA issue key. |
|
@gaurav7261 Applied, thanks a lot! |
