Skip to content
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

Blog post debezium iceberg #714

Closed
wants to merge 20 commits into from
Closed

Blog post debezium iceberg #714

wants to merge 20 commits into from

Conversation

ismailsimsek
Copy link
Contributor

@ismailsimsek ismailsimsek commented Sep 19, 2021

@ismailsimsek ismailsimsek marked this pull request as ready for review September 19, 2021 13:45
@ismailsimsek
Copy link
Contributor Author

cc: @gunnarmorling

@jpechane
Copy link
Contributor

@ismailsimsek Gunnar is OOO, he'll be able to review the post during the next week.

Copy link
Member

@gunnarmorling gunnarmorling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ismailsimsek, thanks a lot! Will take a closer look asap. In the meantime, could you rebase this branch to the latest state of the develop branch? We just merged some preview facility, so ideally we can then take a look at the rendered article in a staging environment. Thx!

_posts/2021-10-01-debezium-server-iceberg.adoc Outdated Show resolved Hide resolved
_posts/2021-10-01-debezium-server-iceberg.adoc Outdated Show resolved Hide resolved
@github-actions
Copy link

⚡️ Deploying PR Preview...

1 similar comment
@github-actions
Copy link

⚡️ Deploying PR Preview...

Copy link
Member

@gunnarmorling gunnarmorling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ismailsimsek, thanks a lot! I've done a full review pass now and put a few comments in place. I like the contents a lot overall; I think in several places there's the need for more contextualization and motivation, so to make clear why you'd want to do these things. There's also a few grammar issues like missing "the"s and casing issues (spark vs. Spark). I can help with those too.

Thanks again and looking forward to the final outcome!

_posts/2021-10-01-debezium-server-iceberg.adoc Outdated Show resolved Hide resolved
_posts/2021-10-01-debezium-server-iceberg.adoc Outdated Show resolved Hide resolved
_posts/2021-10-01-debezium-server-iceberg.adoc Outdated Show resolved Hide resolved
_posts/2021-10-01-debezium-server-iceberg.adoc Outdated Show resolved Hide resolved
_posts/2021-10-01-debezium-server-iceberg.adoc Outdated Show resolved Hide resolved
_posts/2021-10-01-debezium-server-iceberg.adoc Outdated Show resolved Hide resolved
_posts/2021-10-01-debezium-server-iceberg.adoc Outdated Show resolved Hide resolved
_posts/2021-10-01-debezium-server-iceberg.adoc Outdated Show resolved Hide resolved
_posts/2021-10-01-debezium-server-iceberg.adoc Outdated Show resolved Hide resolved
_posts/2021-10-01-debezium-server-iceberg.adoc Outdated Show resolved Hide resolved
@ismailsimsek ismailsimsek marked this pull request as draft October 9, 2021 13:19
@gunnarmorling
Copy link
Member

@ismailsimsek, I think some of the comments still need sorting out, right? Do you have any rough timeline for this? Do you e.g. think we could get this published tomorrow or on Thursday?

@ismailsimsek
Copy link
Contributor Author

@gunnarmorling yes im still updating it and addressing the review comments. If i finish by Thursday i will ping you, latest this weekend it should finish.

@gunnarmorling
Copy link
Member

Ok sounds good. Seems like we could push it out to the website by next week then (I want to avoid Friday, as that will yield less viewers than Thursday or Monday/Tuesday next week). Thanks!

@ismailsimsek
Copy link
Contributor Author

ismailsimsek commented Oct 13, 2021

@gunnarmorling Thank you very much for the review i addressed the comments, extended some sections and reorganized. its ready for another review.

are we concerned with the length of the blog post i feel like this being long blog post compared to others?
and i feel like its being too much Iceberg specific, not sure if any detail about debezium missing?

in term of the time im ok to wait the best time to push it. its up to you. i gues we need to fix the file name before publishing?. once its published im planing to share the link with iceberg and debezium community. and add the link here https://iceberg.apache.org/blogs/

@ismailsimsek ismailsimsek marked this pull request as ready for review October 13, 2021 18:54
Copy link
Member

@gunnarmorling gunnarmorling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ismailsimsek, finally got around to reviewing this; I love the contents, that's really interesting! There were a few issues with wording and grammar, so I moved forward and pushed a commit to address those, also rewriting some parts and shuffling sentences around. Can you please take a look at my changes and let me know whether you are happy with them, or whether you think anything should be modified?

I have also put some other questions/comments for you inline. Depending on your availability for addressing these, we may aim at publishing this article tomorrow (Oct 21), WDYT?

Last question, do you have a Twitter handle I could tag when announcing the post?

Thanks a lot again, looking forward to seeing this wrapped up and published on the blog!

_posts/2021-10-01-debezium-server-iceberg.adoc Outdated Show resolved Hide resolved
This is not optimal for batch processing, especially when a near-realtime data feed is sufficient.
To avoid this problem, it is possible to increase the batch size per commit.

When enabling the `MaxBatchSizeWait` mode, the Iceberg consumer uses Debezium metrics to optimize the batch size. It periodically retrieves the current size of Debezium's internal event queue and waits until it has reached `max.batch.size`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is pretty interesting actually. I'm wondering whether we should support this better out-of-the-box, without making users like you jump through the hoops of fetching the queue size via connector metrics. // CC @jpechane.

_posts/2021-10-01-debezium-server-iceberg.adoc Outdated Show resolved Hide resolved
@ismailsimsek
Copy link
Contributor Author

ismailsimsek commented Oct 20, 2021

Thank you very much @gunnarmorling for the edits and review. Addressed the new comments and added my Twitter handle to authors.yaml file,
im happy with the final version we can publish tomorrow (Oct 21)

@gunnarmorling
Copy link
Member

Thank you very much @gunnarmorling for the edits and review. Addressed the new comments and added my Twitter handle to authors.yaml file

Thanks a lot for the super-fast update, @ismailsimsek! Pushed another commit for some more small changes, and one last question who the "our" refers to in "... in our case ...".

im happy with the final version we can publish tomorrow (Oct 21)

Excellent! In fact, given this is good to go already (with that one last question above remaining), I'd actually put this out later today. More time for folks to come across and read it until the weekend then, which is when interest in these kinds of contents typically is lower.

Thanks again!

@gunnarmorling
Copy link
Member

Excellent! Squashed most of the commits and merged. Should be live on the blog in a few minutes. Thanks a lot, @ismailsimsek, great post and collaboration, really appreciating it!

@github-actions
Copy link

♻️ PR Preview 57be776 has been successfully destroyed since this PR has been closed.

🤖 By surge-preview

@ismailsimsek
Copy link
Contributor Author

great, Thank you @gunnarmorling for the review

@ismailsimsek ismailsimsek deleted the blog_post_debezium-iceberg branch October 20, 2021 13:23
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants