Skip to content

KNOX-2844 - Remote config monitor should not automatically delete local files if they're missing from the DB#680

Merged
zeroflag merged 5 commits intoapache:masterfrom
zeroflag:KNOX-2844-ex
Nov 29, 2022
Merged

KNOX-2844 - Remote config monitor should not automatically delete local files if they're missing from the DB#680
zeroflag merged 5 commits intoapache:masterfrom
zeroflag:KNOX-2844-ex

Conversation

@zeroflag
Copy link
Contributor

@zeroflag zeroflag commented Nov 17, 2022

What changes were proposed in this pull request?

KNOX-2835 introduced an unwanted side effect after enabling the SQL based topology monitor.

The intent of synchronization which was added in KNOX-2835 is:

  • If a descriptor/provider was deleted from the DB the monitor should delete the descriptor/provider from the local file system

But what really happens, implementation wise is that:

  • If a descriptor/provider exists on the file system but doesn't exist in the DB the topology monitor is going to delete it.

These two are not exactly the same. If a user creates a descriptor/provider on the file system, it will be deleted by the topology monitor even though it never existed in the DB. The Zookeeper implementation uses onDeleteEvents which are only triggered when something was previously added but later it was deleted. But the SQL based topology uses polling.

So if one enable this feature with an empty DB, all of the descriptors/providers are going to be deleted.

To address this issue, this patch introduces logical deletes. Deleting a descriptor/provider only sets the delete=true column in the DB. The monitor only deletes a local file if a corresponding record exists in the DB and the delete column is set to true. If something never existed in the DB it'll never be deleted.

The logically deleted records are eventually cleared up to.

How was this patch tested?

<property>
    <name>gateway.service.remoteconfigurationmonitor.impl</name>
    <value>org.apache.knox.gateway.topology.monitor.db.DbRemoteConfigurationMonitorService</value>
</property>
<property>
    <name>gateway.database.type</name>
    <value>mysql</value>
</property>
<property>
    <name>gateway.database.connection.url</name>
    <value>jdbc:mysql://root:root@localhost:3306/knox</value>
</property>

Tested manually with 3 instances of knox. I created a custom descriptor + provider via the admin UI and checked the file content on each host. I deleted a descriptor/provider and checked if the file disappeared from the hosts. I checked the DB content to see the delete=true flag. I waited until the record was physically deleted.
I checked that existing topologies were not effected by the deletes.

@zeroflag zeroflag self-assigned this Nov 17, 2022
@zeroflag zeroflag marked this pull request as draft November 17, 2022 15:39
@zeroflag zeroflag marked this pull request as ready for review November 21, 2022 15:14
zeroflag added 3 commits November 22, 2022 12:00
Copy link
Contributor

@smolnar82 smolnar82 left a comment

Choose a reason for hiding this comment

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

LGTM

@zeroflag zeroflag merged commit 616a4e0 into apache:master Nov 29, 2022
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