Skip to content
This repository has been archived by the owner on Nov 18, 2021. It is now read-only.

DBZ-2552 Refactor LogMinerHelper and SqlUtils #264

Closed
wants to merge 1 commit into from

Conversation

Naros
Copy link
Member

@Naros Naros commented Feb 23, 2021

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

  • Introduced LogWriterFlushStrategy and implementations
  • Moved various functions to LogMinerStreamingChangeEventSource
  • Moved other functions to OracleConnection

* Introduced LogWriterFlushStrategy and implementations
* Moved various functions to LogMinerStreamingChangeEventSource
* Moved other functions to OracleConnection
@gunnarmorling
Copy link
Member

Can you describe a bit what's happening here in the grand scheme of things? Also, we're down to one pending PR, so once that's in, the stars may align for merging this repo into the main one. WDYT?

@Naros
Copy link
Member Author

Naros commented Feb 25, 2021

So the goal with this PR is really code polish.

There are quite a number of methods in LogMinerHelper and SqlUtils utility classes that are only used in a single call location in the code base, mostly in LogMinerStreamingChangeEventSource. The goal is to cleanup these helper classes, move what makes sense to be part of the class where its used for encapsulation purposes and to get closer to a point where these helper classes aren't needed at all as to me they're very bloated & expose multiple concerns.

In addition to, a few methods in the LogMinerHelper class and the streaming event source class use an if/else pattern to handle creating additional database connections to the various nodes in the case of Oracle RAC deployment or using the current database connection for non-RAC deployment. This is used to manage flushing the Oracle LogWriter buffers to guarantee that all changes in the redo logs held in memory get flushed to disk prior to the LogMiner session. I've abstracted this out into a strategy pattern based on LogWriterFlushStrategy as there could be other strategies we could take advantage of in the future; goal being we avoid the need to writing to the database with this "flushing table".

As for migrating the repository to core, I would say we can do that at any point.

@gunnarmorling
Copy link
Member

Closing this for now, will be superseded with another one against the main repo.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants