3.2 Enable backups over CC transaction protocol #9872
Conversation
phughk
changed the title
| int maxPages = 1024; | ||
| PageSwapperFactory pageSwapperFactory = new SingleFilePageSwapperFactory(); | ||
| pageSwapperFactory.setFileSystemAbstraction( fileSystemAbstraction ); | ||
| return new MuninnPageCache( pageSwapperFactory, maxPages, cachePageSize, pageCacheTracer, pageCursorTracerSupplier ); |
There was a problem hiding this comment.
Are you intentionally ignoring the page cache configurations in neo4j.conf? In doing so, we can no longer do an online backup directly to block device storage, but have to change the procedure to back up to file storage and then import onto block device storage.
There was a problem hiding this comment.
Not intentionally - I will fix that, thanks!
phughk
force-pushed
the
3.2-backupRaft
branch
12 times, most recently
from
8258a5a
to
f670ff2
Compare
phughk
force-pushed
the
3.2-backupRaft
branch
11 times, most recently
from
65e990e
to
dd9954f
Compare
| } | ||
| return (E) e; | ||
| } | ||
| throw new RuntimeException( "The code expected to fail hasn't failed" ); |
There was a problem hiding this comment.
Maybe you want to throw AssertionErrors instead.
There was a problem hiding this comment.
Addressed in next push
| } | ||
| } ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Is it always enabled, and the extension is registered automatically?
There was a problem hiding this comment.
It should always be disabled for CC (there is no need in maintaining it, since the neo4j-admin will be the same version as the cc running). But you are saying it is registered automatically? That is what I need to avoid (@Ignore in OnlineBackupCommandCcIt)
There was a problem hiding this comment.
Resolved - CC (core+RR) no longer have the backup protocol
| import org.neo4j.test.ProcessStreamHandler; | ||
| import org.neo4j.test.rule.TestDirectory; | ||
|
|
||
| public class JvmRunner |
There was a problem hiding this comment.
Seems pretty backup specific. Maybe just add them as more static methods to org.neo4j.backup.util.TestHelpers?
There was a problem hiding this comment.
Because it contains @Rule TestDirectory then it is expected to be extended from. The method that doesn't require NEO4J_HOME depends on the TestDirectory. Also it had to be moved to enterprise/kernel because there are tests in causal clustering (if I recall) that use this as well. Its a maven tree thing essentially
There was a problem hiding this comment.
OnlineBackupCommandCcIT is the only test that uses that feature of it, and it could just as well have its own TestDirectory.
phughk
force-pushed
the
3.2-backupRaft
branch
2 times, most recently
from
23c3f70
to
1c54ab5
Compare
|
You got a couple of compilation failures. |
phughk
force-pushed
the
3.2-backupRaft
branch
5 times, most recently
from
706e39e
to
35b812c
Compare
phughk
force-pushed
the
3.2-backupRaft
branch
2 times, most recently
from
aaa2742
to
0b74576
Compare
|
Conflicts are being resolved in linked PR |
phughk
force-pushed
the
3.2-backupRaft
branch
3 times, most recently
from
2a5387e
to
29813de
Compare
phughk
force-pushed
the
3.2-backupRaft
branch
3 times, most recently
from
f95a881
to
61320ee
Compare
Before this commit, CC is dependant on backup. Applying this commit will shuffle dependencies around to be more sensible
Backup tool works over transaction protocol CC (core, RR) have the backup protocol disabled PERMANENTLY (switching option won't change anything) HA backup protocol still supported as it had before
Moved JvmRunner code to TestHelpers Moved TestHelpers to enterprise kernel Resolved CC starting Backup protocol unintentionally.
phughk
force-pushed
the
3.2-backupRaft
branch
from
61320ee
to
b63950d
Compare
Currently we have a separate protocol for backups. This protocol is used in HA but also in the backup tool (neo4j-admin).
Introducing this change enables
neo4j-admin backupto work with CC endpoints without any change of configuration.