DBZ-7871 Change logging level for Found previous partition offset

[rkudryashov]

https://issues.redhat.com/browse/DBZ-7871

[jpechane]

I definitely don't want to remove it globally. The same method is used for initial idnetification of existing offsets by all connectors and it is IMHO important that the message is logged at INFO level. I'd be OK with intorducing a flag that will decide whether the logging is at INFO or DEBUG/TRACE level and intorudce a new method that would accept that flag. By default INFO will be used and his instance can override it.

[mfvitale]

I definitely don't want to remove it globally. The same method is used for initial idnetification of existing offsets by all connectors and it is IMHO important that the message is logged at INFO level. I'd be OK with intorducing a flag that will decide whether the logging is at INFO or DEBUG/TRACE level and intorudce a new method that would accept that flag. By default INFO will be used and his instance can override it.

@jpechane , What do you think to remove the log side effect out of this method and then log it on the task? So any other use of that method will require no previous knowledge.

Majority of connectors (Oracle, MariaDB, MySQL, POstgreSQL) already log again this info

debezium/debezium-connector-postgres/src/main/java/io/debezium/connector/postgresql/PostgresConnectorTask.java

Lines 164 to 169 in c09c2c6

	if (previousOffset == null) {
	LOGGER.info("No previous offset found");
	}
	else {
	LOGGER.info("Found previous offset {}", previousOffset);
	}

Maybe this is a good chance to align the behavior and remove these duplicate logs.

[Naros]

While they do @mfvitale, I can see certain situations where knowing what the offset is per partition could be useful in the case of SQL Server where it stores offsets across partitions, so I wouldn't recommend removing it. What makes it more complicated as removing the logging would break behavior for MongoDB which has a very different way of logging this phase for users.

I think the path of least resistance here would be as Jiri mentioned, pass a flag from PG where we do this special call in the commit method to bypass logging like we do for replica identity and schema refreshes.

As a follow-up nit-pick, all those if/else you mentioned, I'd actually recommend a new method be added to the base source task that those connectors could call that logs this consistently to reduce the logic in all the tasks start-up methods that currently use the if/else block.

[mfvitale]

While they do @mfvitale, I can see certain situations where knowing what the offset is per partition could be useful in the case of SQL Server where it stores offsets across partitions, so I wouldn't recommend removing it. What makes it more complicated as removing the logging would break behavior for MongoDB which has a very different way of logging this phase for users.

If when you say "knowing" you mean logging that info, why to not log it on the task looking at the Offsets returned by getPreviousOffsets?

Maybe putting these logging into a separate method as per your suggestion

As a follow-up nit-pick, all those if/else you mentioned, I'd actually recommend a new method be added to the base source task that those connectors could call that logs this consistently to reduce the logic in all the tasks start-up methods that currently use the if/else block.

I think the path of least resistance here would be as Jiri mentioned, pass a flag from PG where we do this special call in the commit method to bypass logging like we do for replica identity and schema refreshes.

Yeah this is the fast and easy, I am just trying to figure out if there can be better options that simplifies the code.

[jpechane]

@mfvitale WDYT? 04372ac

[mfvitale]

@mfvitale WDYT? 04372ac

Sounds good. Just leaved a nitpick comment.

[jpechane]

@mfvitale Comment added