CASSANDRA-16619: Add originating host ID to sstable metadata (trunk) #978
Conversation
blambov
left a comment
blambov
left a comment
There was a problem hiding this comment.
LGTM.
Please create a new ticket to generalize this mechanism to handle other pieces of metadata that are only meaningful for the originating node.
jacek-lewandowski
force-pushed
the
CASSANDRA-16619-4.0
branch
5 times, most recently
from
dc022b8 to
0cc6550
Compare
- md, me, nb sstables - Update version to nb, update MetadataSerializerTest - add missing test cases in MetadataSerializerTest Co-authored-by: Jakub Zytka <jakub.zytka@datastax.com> Co-authored-by: Jacek Lewandowski <jacek.lewandowski@datastax.com>
jacek-lewandowski
force-pushed
the
CASSANDRA-16619-4.0
branch
from
0cc6550 to
0b88659
Compare
| } | ||
|
|
||
| if (!skippedSSTables.isEmpty()) { | ||
| logger.warn("Origin of {} sstables is unknown or doesn't match the local node; commitLogIntervals for them were ignored", skippedSSTables.size()); |
There was a problem hiding this comment.
We can get a big number here after upgrade. Perhaps it makes sense to add some info about that - that this is nothing to worry about?
There was a problem hiding this comment.
hmm... I think I can add more information
| hasCommitLogLowerBound = version.compareTo("mb") >= 0; | ||
| hasCommitLogIntervals = version.compareTo("mc") >= 0; | ||
| hasAccurateMinMax = version.compareTo("md") >= 0; | ||
| hasOriginatingHostId = version.matches("(m[e-z])|(n[b-z])"); |
There was a problem hiding this comment.
o* versions are good too ;)
Noone's gonna remember to change this if o is ever introduced/
I think we need to make some comparisons with version.compareTo("nb") >= 0 being the last
| testMetricsWithStreamingToTwoNodes(true); | ||
| } | ||
|
|
||
| private int getNumberOfSSTables(Cluster cluster, int node) { |
There was a problem hiding this comment.
could you please elaborate what do these changes fix?
There was a problem hiding this comment.
hehe... this is in-jvm distributed test. I assumed exact number of sstables in various stages of the test case. After adding that sentinel to commit log replayer, there is one extra sstable created because local host id is unknown. And it is unknown because the server is running in a kind of degraded mode (for speeding up tests, etc).
| * </p> | ||
| */ | ||
| @Ignore | ||
| // @Ignore |
There was a problem hiding this comment.
I believe we want this @Ignore back, once the serialization samples have been generated
There was a problem hiding this comment.
ah yes, indeed
|
|
||
| @Test | ||
| public void testMaReadMc() throws IOException | ||
| private void testVersions(String... versions) throws Throwable |
There was a problem hiding this comment.
I propose testOldReadsNew name (and similarly for testOldReadsNewMVersions and testOldReadsNewNVersions). This is the crux of the test, but with the current name it's hidden in the very details of the implementation.
|
Merged manually |
Optimize the `ChunkCache#invalidateFile` method by introducing a map that keeps track of the file to key set in the `ChunkCache`. The current solution is costly, as it iterates over the whole cache.
4 participants
No description provided.