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

Try to fix add new table crash #1

Open
wants to merge 1 commit into
base: 1.9
Choose a base branch
from
Open

Conversation

yatsvic
Copy link
Owner

@yatsvic yatsvic commented Aug 26, 2022

We will have overhead. If database.history.store.only.monitored.tables.ddl is set to true on each connector start it will additionally load tables list from database.

p.s. store.only.captured.tables.ddl is deprecated, should use store.only.captured.tables.ddl instead

@yatsvic yatsvic force-pushed the mysql-new-table-fix branch from d788aa3 to 088b52b Compare August 26, 2022 13:51
@yatsvic yatsvic changed the title draft Try to fix add new table crash Aug 26, 2022
@wadesherman
Copy link

this is great - i will test this locally

private boolean shouldRecaptureSchemaSnapshot(MySqlPartition partition){
try (MySqlSnapshotContext ctx = (MySqlSnapshotContext)prepare(partition)){
determineCapturedTables(ctx);
boolean result = !databaseSchema.tableIds().containsAll(ctx.capturedSchemaTables);

Choose a reason for hiding this comment

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

do you think it is feasible to just call createSchemaEventsForTables right here?

Copy link
Owner Author

@yatsvic yatsvic Aug 30, 2022

Choose a reason for hiding this comment

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

@wadesherman I have tried

    private void ensureSchemaForNewTables(MySqlPartition partition) {
        try (MySqlSnapshotContext snapshotContext = (MySqlSnapshotContext) prepare(partition)) {
            determineCapturedTables(snapshotContext);
            boolean newTables = !databaseSchema.tableIds().containsAll(snapshotContext.capturedSchemaTables);
            LOGGER.info("new tables detected: " + newTables);
            if (newTables) {
                ChangeEventSourceContext sourceContext = new ChangeEventSourceContext() {

                    @Override
                    public boolean isRunning() {
                        return true;
                    }
                };

                SnapshottingTask task = new SnapshottingTask(true, false);
                createSchemaChangeEventsForTables(sourceContext, snapshotContext, task);
            }

        }
        catch (Exception e) {
            LOGGER.error("Failed to recapture new tables schemas.", e);
            throw new RuntimeException(e);
        }
    }

    @Override
    protected SnapshottingTask getSnapshottingTask(MySqlPartition partition, MySqlOffsetContext previousOffset) {

        boolean snapshotSchema = true;
        boolean snapshotData = true;

        // found a previous offset and the earlier snapshot has completed
        if (previousOffset != null && !previousOffset.isSnapshotRunning()) {
            LOGGER.info("A previous offset indicating a completed snapshot has been found. Neither schema nor data will be snapshotted.");
            snapshotSchema = databaseSchema.isStorageInitializationExecuted();
            if (!snapshotSchema && databaseSchema.storeOnlyCapturedTables()) {
                ensureSchemaForNewTables(partition);
            }
            snapshotData = false;
        }
...

But it doesn't produce schema messages. Looks like that something is not inititialized. I think that solution in this branch is good. The only one thing is memory overhead: it recapture all tables schemas. But it uses standard snapshot procedure with locks and interruption on connector stop.

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.

2 participants