You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2017/10/06 14:19:07 UTC
[03/12] cassandra git commit: Fix handling of cells for removed
column when reading legacy sstables
Fix handling of cells for removed column when reading legacy sstables
Patch by Sylvain Lebresne; reviewed by Robert Stupp for CASSANDRA-13939
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/5378ba27
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/5378ba27
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/5378ba27
Branch: refs/heads/trunk
Commit: 5378ba27ff66b4f39e34d613f99b0f2eddf7990d
Parents: c074412
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Thu Oct 5 10:30:56 2017 +0200
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Fri Oct 6 16:12:56 2017 +0200
----------------------------------------------------------------------
.../org/apache/cassandra/db/LegacyLayout.java | 70 +++++++++++++-------
.../cassandra/db/UnfilteredDeserializer.java | 29 +++++---
2 files changed, 67 insertions(+), 32 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5378ba27/src/java/org/apache/cassandra/db/LegacyLayout.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/LegacyLayout.java b/src/java/org/apache/cassandra/db/LegacyLayout.java
index 40b9fd3..3ba96a6 100644
--- a/src/java/org/apache/cassandra/db/LegacyLayout.java
+++ b/src/java/org/apache/cassandra/db/LegacyLayout.java
@@ -649,7 +649,7 @@ public abstract class LegacyLayout
boolean foundOne = false;
LegacyAtom atom;
- while ((atom = readLegacyAtom(metadata, in, false)) != null)
+ while ((atom = readLegacyAtomSkippingUnknownColumn(metadata,in)) != null)
{
if (atom.isCell())
{
@@ -672,6 +672,23 @@ public abstract class LegacyLayout
return foundOne ? builder.build() : Rows.EMPTY_STATIC_ROW;
}
+ private static LegacyAtom readLegacyAtomSkippingUnknownColumn(CFMetaData metadata, DataInputPlus in)
+ throws IOException
+ {
+ while (true)
+ {
+ try
+ {
+ return readLegacyAtom(metadata, in, false);
+ }
+ catch (UnknownColumnException e)
+ {
+ // Simply skip, as the method name implies.
+ }
+ }
+
+ }
+
private static Row getNextRow(CellGrouper grouper, PeekingIterator<? extends LegacyAtom> cells)
{
if (!cells.hasNext())
@@ -1020,29 +1037,36 @@ public abstract class LegacyLayout
};
}
- public static LegacyAtom readLegacyAtom(CFMetaData metadata, DataInputPlus in, boolean readAllAsDynamic) throws IOException
+ public static LegacyAtom readLegacyAtom(CFMetaData metadata, DataInputPlus in, boolean readAllAsDynamic)
+ throws IOException, UnknownColumnException
{
- while (true)
- {
- ByteBuffer cellname = ByteBufferUtil.readWithShortLength(in);
- if (!cellname.hasRemaining())
- return null; // END_OF_ROW
-
- try
- {
- int b = in.readUnsignedByte();
- return (b & RANGE_TOMBSTONE_MASK) != 0
- ? readLegacyRangeTombstoneBody(metadata, in, cellname)
- : readLegacyCellBody(metadata, in, cellname, b, SerializationHelper.Flag.LOCAL, readAllAsDynamic);
- }
- catch (UnknownColumnException e)
- {
- // We can get there if we read a cell for a dropped column, and ff that is the case,
- // then simply ignore the cell is fine. But also not that we ignore if it's the
- // system keyspace because for those table we actually remove columns without registering
- // them in the dropped columns
- assert metadata.ksName.equals(SystemKeyspace.NAME) || metadata.getDroppedColumnDefinition(e.columnName) != null : e.getMessage();
- }
+ ByteBuffer cellname = ByteBufferUtil.readWithShortLength(in);
+ if (!cellname.hasRemaining())
+ return null; // END_OF_ROW
+
+ try
+ {
+ int b = in.readUnsignedByte();
+ return (b & RANGE_TOMBSTONE_MASK) != 0
+ ? readLegacyRangeTombstoneBody(metadata, in, cellname)
+ : readLegacyCellBody(metadata, in, cellname, b, SerializationHelper.Flag.LOCAL, readAllAsDynamic);
+ }
+ catch (UnknownColumnException e)
+ {
+ // We legitimately can get here in 2 cases:
+ // 1) for system tables, because we've unceremoniously removed columns (without registering them as dropped)
+ // 2) for dropped columns.
+ // In any other case, there is a mismatch between the schema and the data, and we complain loudly in
+ // that case. Note that if we are in a legit case of an unknown column, we want to simply skip that cell,
+ // but we don't do this here and re-throw the exception because the calling code sometimes has to know
+ // about this happening. This does mean code calling this method should handle this case properly.
+ if (!metadata.ksName.equals(SystemKeyspace.NAME) && metadata.getDroppedColumnDefinition(e.columnName) == null)
+ throw new IllegalStateException(String.format("Got cell for unknown column %s in sstable of %s.%s: " +
+ "This suggest a problem with the schema which doesn't list " +
+ "this column. Even if that column was dropped, it should have " +
+ "been listed as such", metadata.ksName, metadata.cfName, UTF8Type.instance.compose(e.columnName)), e);
+
+ throw e;
}
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5378ba27/src/java/org/apache/cassandra/db/UnfilteredDeserializer.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/UnfilteredDeserializer.java b/src/java/org/apache/cassandra/db/UnfilteredDeserializer.java
index ea65633..0aa5741 100644
--- a/src/java/org/apache/cassandra/db/UnfilteredDeserializer.java
+++ b/src/java/org/apache/cassandra/db/UnfilteredDeserializer.java
@@ -274,16 +274,26 @@ public abstract class UnfilteredDeserializer
private LegacyLayout.LegacyAtom readAtom()
{
- try
- {
- long pos = currentPosition();
- LegacyLayout.LegacyAtom atom = LegacyLayout.readLegacyAtom(metadata, in, readAllAsDynamic);
- bytesReadForNextAtom = currentPosition() - pos;
- return atom;
- }
- catch (IOException e)
+ while (true)
{
- throw new IOError(e);
+ try
+ {
+ long pos = currentPosition();
+ LegacyLayout.LegacyAtom atom = LegacyLayout.readLegacyAtom(metadata, in, readAllAsDynamic);
+ bytesReadForNextAtom = currentPosition() - pos;
+ return atom;
+ }
+ catch (UnknownColumnException e)
+ {
+ // This is ok, see LegacyLayout.readLegacyAtom() for why this only happens in case were we're ok
+ // skipping the cell. We do want to catch this at this level however because when that happen,
+ // we should *not* count the byte of that discarded cell as part of the bytes for the atom
+ // we will eventually return, as doing so could throw the logic bytesReadForNextAtom participates in.
+ }
+ catch (IOException e)
+ {
+ throw new IOError(e);
+ }
}
}
@@ -407,6 +417,7 @@ public abstract class UnfilteredDeserializer
saved = null;
iterator.clearState();
lastConsumedPosition = currentPosition();
+ bytesReadForNextAtom = 0;
}
// Groups atoms from the input into proper Unfiltered.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org