fix(server): keep the hbase meta table when clearing a graph - #3176
Open
bitflicker64 wants to merge 1 commit into
Open
fix(server): keep the hbase meta table when clearing a graph#3176bitflicker64 wants to merge 1 commit into
bitflicker64 wants to merge 1 commit into
Conversation
Clearing a graph (DELETE /graphs/{name}/clear) calls truncate() on each
of the three stores. HbaseSystemStore.tableNames() appends the meta
table, so the truncate also wiped the backend version that init() had
written there, and the next start of the server failed the version
check with "The backend store version is inconsistent" (apache#2209).
Add HbaseStore.tableNamesToTruncate(), all the tables by default, and
override it in HbaseSystemStore to leave out the meta table only. The
schema store still truncates its counters table, so schema ids restart
from the beginning after a clear, as they do on RocksDB. The rollback
path re-enables the same tables it disabled.
Add a core test that clears a graph and checks that the version check
still passes, the schema and the data are gone, and the schema ids are
handed out again from the start. It runs on every backend of the CI
matrix.
Supersedes apache#2911 by LYD031106.
closes apache#2209
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3176 +/- ##
============================================
+ Coverage 32.66% 37.74% +5.08%
- Complexity 5500 6535 +1035
============================================
Files 789 800 +11
Lines 67703 68886 +1183
Branches 8945 9147 +202
============================================
+ Hits 22116 26004 +3888
+ Misses 42983 39832 -3151
- Partials 2604 3050 +446 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose of the PR
closes #2209
Replaces #2911 by @LYD031106, whose head branch lives on a personal fork and has been inactive since January. The bug is still live on master.
DELETE /graphs/{name}/clearends inHbaseStore.truncate()for each of the three stores.HbaseSystemStore.tableNames()appends the meta table, so the truncate also wiped the backend version thatinit()writes there. The next start of the server then failedBackendStoreInfo.checkVersion()with "The backend store version is inconsistent", and the known workarounds were to re-create the backend database and init the graph again, or to insert the version row by hand.Main Changes
HbaseStore: newtableNamesToTruncate(), returningtableNames()by default.truncate()uses it, and the rollback path (enableTables) re-enables exactly the tables it disabled instead of iteratingtableNames()again.HbaseSystemStore: overridestableNamesToTruncate()to return every table but the meta table. Only the meta table is kept. The schema store is untouched, so its counters table is still truncated and schema ids restart from the beginning after a clear, the same as RocksDB (clear(false)+init()). The version row is never deleted and no storeinit()runs after the truncate, so the version row is never rewritten and there is no window during the clear in which the version check could fail.truncateBackend()waits for the running tasks and takes the graph lock, the same asclearBackend()andinitBackend(); a concurrency test for the clear is out of scope for this fix.MultiGraphsTest.testTruncateBackendKeepsVersionAndResetsSchemaIds: opens a graph, writes schema and data, callstruncateBackend(), then checks thatcheckVersion()still passes, the schema and the data are gone, a vertex label created again gets the same id as before, and the graph is usable. It is part ofCoreTestSuite, so it runs on memory, rocksdb and hbase inserver-ci.yml. It is skipped on hstore (pd-store-ci.yml): there the schema lives in PD meta, whichtruncateBackend()does not clear (onlyclearBackend()does), andIdCounterhands out ids from a cached range, so the schema and id checks do not apply. The HBase change does not touch hstore.Differences from #2911, on purpose:
this.tablesonHugeType.META, butMETAis never a key of that map (the meta table is added inHbaseSystemStore.tableNames()), so the filter was a no-op. The fix worked only because the new method readthis.tablesdirectly and skipped thetableNames()overrides. That also skippedHbaseSchemaStore.tableNames(), so the counters table was no longer truncated and schema ids kept growing after a clear, which the Critical review on fix(server): ensure graph clear API does not clear meta table for hbase #2911 (only the meta table should be kept) had asked to avoid. Here the exclusion is explicit in the system store and the schema store is not touched.BaseHbaseUnitTest/HbaseUnitTest) is not carried over. It talked to HBase through raw sessions, asserted only the version and three data rows, could not see the counters regression, swallowed setup failures with a warning, and on a setup failure ran teardown twice, withprovider.truncate()(the method under test) outside any try/catch. The core test above covers the same behaviour through the public graph API on every backend, without HBase-specific plumbing, and it also covers the counters.clear(false)+init()intruncate(): it does not have this bug, and unifying the backends on one approach, as raised on fix(server): ensure graph clear API does not clear meta table for hbase #2911, is a separate refactor. The MySQL backend that the sibling PR fix(server): prevent graph clear API from clearing meta table in MySQL backend storage #2888 targets was removed from master by chore: remove legacy backends #3116.run-unit-test.shrunsUnitTestSuiteon the memory leg only, so a backend-specific test there never sees HBase, which is why fix(server): ensure graph clear API does not clear meta table for hbase #2911 needed theAssume.run-core-test.shrunsCoreTestSuitewith-P core-test,$BACKENDonce per backend of thebuild-servermatrix ofserver-ci.yml, afterinstall-backend.sh hbasehas started HBase 2.6.5. That is why the new test lives inCoreTestSuiteand needs noAssume.Verifying these changes
MultiGraphsTest.testTruncateBackendKeepsVersionAndResetsSchemaIds(new)hbase-site.xmlapart from the data directories), JDK 11,-P core-test,hbase: on master without theHbaseStorechange the test fails right after the truncate, atcheckVersion()(stored versionnull); with the change it passes. FullCoreTestSuiteon hbase with the change: 805 run, 0 failures, 0 errors, 52 skipped, 795 s. The skips are the existing hstore-only gates and HBase feature gates (aggregate and olap properties, scan, ramtable, CONTAINS queries): the hbase job on master at 321ba4d reports 804 run and the same 52 skipped.MultiGraphsTest: 11 run, 0 failures, 0 errors, 2 skipped (the two existing hstore-gated tests) on both backends.mvn checkstyle:checkonhugegraph-hbaseandhugegraph-test: 0 violations.Does this PR potentially affect the following parts?
Documentation Status
Doc - No Need