Repository navigation
Conversation
Fixes JanusGraph#5097 StringSerializer compresses a string longer than 16,000 UTF-16 units with GZIP, from String.getBytes(UTF_8). UTF-8 has no encoding for an unpaired surrogate, a unit from U+D800 to U+DFFF which isn't half of a surrogate pair, and getBytes writes a question mark in its place. So such a string came back from storage with question marks in place of its unpaired surrogates, while a shorter string, which StringSerializer encodes unit by unit, keeps them. Strings which differ only there were stored alike: the values of a SET property became one, and a unique composite index took such strings for duplicates. The bytes to compress are now WTF-8: UTF-8, except that an unpaired surrogate takes the three bytes which UTF-8 would give its code point. A string without surrogates is still encoded by getBytes, byte for byte as before; one with a surrogate is encoded by a loop, which gives getBytes's bytes for every other character and every pair. Decompression decodes those three bytes back into the surrogate. Java's UTF-8 decoder reads them as U+FFFD, so the bytes are searched for them only when the decoded string holds U+FFFD. The compressor id stays GZIP. Strings written before read as before, since Java's encoder never writes the bytes of a surrogate. An earlier version reads a string written now with U+FFFD in place of each unpaired surrogate, where a new compressor id would have made the string unreadable to it. The stream which decompression reads the stored bytes through now fills GZIPInputStream's buffer in one call of read(byte[], int, int), where InputStream's implementation of it called read() for each byte. Decompression also closes the GZIPInputStream, which frees the native memory of its Inflater at once. Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com> Co-authored-by: Oleksandr Porunov <alexandr.porunov@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open findings
What changed in this PR
This PR fixes loss of unpaired UTF-16 surrogates when JanusGraph compresses long strings (>16,000 UTF-16 units) by switching the compressed representation from Java UTF-8 bytes (which replace unpaired surrogates with ?) to WTF-8 bytes and decoding them back on read, preserving backward compatibility.
Changes:
- Update
StringSerializerGZIP compression to write WTF-8 bytes for long strings and decode them back on decompress; also improve theInputStream.read(byte[], int, int)path and closeGZIPInputStream. - Add targeted tests to validate surrogate preservation, backward compatibility, and byte-level stability for valid strings.
- Add
assertSameCharshelper for reliable assertion messages when strings contain unpaired surrogates; document behavior in changelog.
| File | Description |
|---|---|
| janusgraph-core/src/main/java/org/janusgraph/graphdb/database/serialize/attribute/StringSerializer.java | Switch long-string compression payload to WTF-8 and decode on read; optimize GZIP read path and ensure stream closure. |
| janusgraph-test/src/test/java/org/janusgraph/graphdb/serializer/SerializerTest.java | Add comprehensive unit tests for long strings with/unpaired surrogates and backward compatibility byte formats. |
| janusgraph-backend-testutils/src/main/java/org/janusgraph/testutil/JanusGraphAssert.java | Add string assertion helper that reports the first differing UTF-16 unit safely. |
| janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphTest.java | Add integration test ensuring composite unique index + SET cardinality behave correctly with long strings containing unpaired surrogates. |
| docs/changelog.md | Document behavior change and operational caveats for mixed-version clusters and indexed keys. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
2 open findings
3 resolved since last review
🧠 Review effort: Lite
… keys Fixes JanusGraph#5109 LuceneIndex removes a value of a key of several values from a document by comparing the string which convertToStringValue gives for the value with the stored values of the field, as Lucene reads them back. For two kinds of values that string wasn't the stored one: - A float is stored as the double which it widens to, whose decimal string differs from the float's for most floats: "0.1" against "0.10000000149011612" for 0.1f. The zeros, NaN, the infinities and floats such as 1.5f have the same string either way. - A string is stored in UTF-8, which has no encoding for an unpaired surrogate, so Lucene writes U+FFFD in its place and reads that back. Such a value stayed in the document when it was removed, and eq, the ranges and the text predicates kept finding the element by it, also once the element held no value of the key. So did the value which an edge held before it was set to another one. And a value given again to a SET key whose document held it was held twice, as the addition didn't find the entry which it replaces. convertToStringValue now gives a number as buildStoreField stores it, a whole number as a long and a decimal as a double, and a string as it reads back from UTF-8. What Lucene writes doesn't change. A string longer than 16,000 UTF-16 units is removed only where storage gives it back with its unpaired surrogates, which JanusGraph#5098 makes it do: master reads it back with question marks in their place. Values which Lucene stores alike, such as strings which differ only in an unpaired surrogate, or in one and U+FFFD, now always share one entry where a SET key holds both, as two instants of one millisecond did: the addition of the second replaces the entry of the first, as it compares equal now, so removing either leaves the other unfound. Values which an earlier version left in a document stay there until the document is written whole again: a reindex writes the document of each element which holds a value of the index, and leaves the others. Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com> Co-authored-by: Oleksandr Porunov <alexandr.porunov@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
2 open issues
2 resolved since last review
Review effort: Lite
| public static void assertSameChars(String expected, String actual) { | ||
| assertNotNull(actual); | ||
| assertTrue(expected.equals(actual), () -> { | ||
| int i = 0; | ||
| while (i < expected.length() && i < actual.length() && expected.charAt(i) == actual.charAt(i)) i++; | ||
| return String.format("Expected %d chars but was %d, which first differ at %d: expected %s but was %s", | ||
| expected.length(), actual.length(), i, unitAt(expected, i), unitAt(actual, i)); | ||
| }); | ||
| } |
| `I` written by a JVM with a Turkish or Azerbaijani default locale. A mixed index on Elasticsearch whose store name holds | ||
| an `I` or a dotted capital `İ`, and whose index such a JVM created, needs one too. | ||
|
|
||
| ##### Strings longer than 16,000 characters keep their unpaired surrogates |


Fixes #5097
StringSerializercompresses a string longer than 16,000 UTF-16 units (LONG_COMPRESSION_THRESHOLD) with GZIP, fromString.getBytes(StandardCharsets.UTF_8). UTF-8 has no encoding for an unpaired surrogate, a unit from U+D800 to U+DFFF which isn't half of a surrogate pair, andgetByteswrites a question mark in its place. So such a string came back from storage with question marks in place of its unpaired surrogates, while a shorter string keeps them:StringSerializerencodes it itself, each unit as one to three bytes. Strings which differ only there were stored alike: the values of aSETproperty, which aSETkeeps in the keys of its columns, became one, and the key of a composite index entry held the question marks too, so a unique index took such strings for duplicates. The same held for the long elements of aString[]property and the long text of a JSON property, which the same serializer writes. This PR compresses the WTF-8 bytes of a string instead, which differ from its UTF-8 bytes only where it has an unpaired surrogate, and decodes them back, under the same compressor id. So every string without unpaired surrogates is written byte for byte as before, strings written before read as before, and earlier versions still read the strings written now.Changes
StringSerializer, the GZIP compressor:encodeWtf8(new) gives the bytes to compress.getBytes(StandardCharsets.UTF_8), as before, once a loop has looked for the first surrogate.getBytes's bytes, and an unpaired surrogate as the three bytes which UTF-8 would give its code point,ED A0 80toED BF BF. That is WTF-8, which the WTF-8 specification defines.decodeWtf8(new) decodes the decompressed bytes withnew String(bytes, StandardCharsets.UTF_8), as before.ED, thenA0toBF, then a continuation byte. UTF-8 hasEDonly before80to9F. A counted loop searches the bytes forED.new Stringas well.GZIPInputStreamreads the stored bytes from now overridesread(byte[], int, int), through whichGZIPInputStreamfills its buffer.InputStream's own implementation of it callsread()for each byte. The override checks its arguments asInputStream's does. See the notes for what it saves.decompresscloses itsGZIPInputStream, which frees the native memory of itsInflaterat once, where master left that to the garbage collector. Closing it closes the stream it reads the stored bytes from, which does nothing.JanusGraphAssert.assertSameChars(new): asserts that two strings are equal, and where they aren't, tells the first UTF-16 unit which differs. Surefire cuts a message off at an unpaired surrogate:assertEqualson these strings reportedexpected: <and nothing more.Tests
SerializerTest(janusgraph-test), five new tests:testLongStringsKeepUnpairedSurrogates: strings of 16,009 to 16,012 units, of ASCII, Latin-1, Cyrillic and CJK characters and surrogate pairs, with unpaired surrogates at the start, in the middle and at the end: high and low ones, the first and last of each, two in a row, next to a pair, and next to a question mark and U+FFFD. They read back as written, as do short strings with the same surrogates, a string of 16,000 units ending in one (not compressed) and one of 16,001, and 16,001 unpaired low surrogates.testRandomLongStringsKeepUnpairedSurrogates: 100 random strings of 16,001 to 17,001 units, of units from every range, unpaired surrogates included, and supplementary characters, read back as written. Their bytes, decoded as earlier versions decode them, give the strings with U+FFFD in place of each unpaired surrogate.testUnpairedSurrogatesOfLongStringsAreWrittenAsTheirCodePoints: the bytes which a string with unpaired U+DC00, U+D800 and U+DFFF and a pair is compressed from areED B0 80,ED A0 80,ED BF BFandF0 9F 98 80around the UTF-8 of the rest, and the compressor id is 1.testLongStringsWithoutUnpairedSurrogatesAreWrittenAsBefore: six long strings without unpaired surrogates (ASCII, mixed scripts with pairs, U+FFFD and question marks, CJK, only pairs, random) are written byte for byte as master wrote them: the length of the compressed bytes and the compressor id, thengetBytes(StandardCharsets.UTF_8)compressed withGZIPOutputStream.testLongStringsWrittenBeforeAreReadAsBefore: bytes written as master writes them read back as before, with question marks in place of the unpaired surrogates, and U+FFFD and the question marks of the string kept.JanusGraphTest.testLongStringsKeepUnpairedSurrogates(every storage backend), with a unique composite index on the key:SETkey which differ only in their last unit: U+D800, U+DC00 or a question mark.On master's code:
JanusGraphTest.testLongStringsKeepUnpairedSurrogatesfails on inmemory: "Expected 16005 chars but was 16005, which first differ at 0: expected U+DC00 but was U+003F". Past that check, master finds the vertex by the string with question marks too, its unique index rejects the second vertex with aSchemaViolationException, and the vertex keeps one value of theSET(on BerkeleyJE; inmemory keeps three copies of the one ending in a question mark): the same steps show these in programs run on master.SerializerTest,testLongStringsKeepUnpairedSurrogatesandtestRandomLongStringsKeepUnpairedSurrogatesfail the same way.testUnpairedSurrogatesOfLongStringsAreWrittenAsTheirCodePointsfails as the bytes are 24,019 rather than 24,025: three unpaired surrogates take one byte each instead of three.testLongStringsWithoutUnpairedSurrogatesAreWrittenAsBeforeandtestLongStringsWrittenBeforeAreReadAsBefore, pass on master.Mutation checks, each run on the unit tests and on the graph test on inmemory:
A0toAFalone as the second byte, so high surrogates only: the low surrogates come back as U+FFFD, which fails the graph test and two unit tests.testLongStringsWithoutUnpairedSurrogatesAreWrittenAsBeforefinds other bytes for the strings with pairs,testUnpairedSurrogatesOfLongStringsAreWrittenAsTheirCodePointsfinds 26,695 bytes, and earlier versions would read each pair as two U+FFFD, which failstestRandomLongStringsKeepUnpairedSurrogates.read(byte[], int, int)not counting the bytes it read, or reading one byte too many: four of the new tests andtestStringCompressionfail withArrayIndexOutOfBoundsExceptionas the stream reads past the stored bytes, andtestSerializationMixturereads the next value from the wrong place. The first also fails the graph test.getByteswhen the first surrogate is half of a pair: a string with a pair before an unpaired surrogate loses that one, which failstestLongStringsKeepUnpairedSurrogatesandtestRandomLongStringsKeepUnpairedSurrogates.getBytes, every test passes,testLongStringsWithoutUnpairedSurrogatesAreWrittenAsBeforeincluded: the loop givesgetBytes's bytes for those six strings, as it follows the JDK's UTF-8 encoder branch for branch, apart from unpaired surrogates. A string with a pair but no unpaired surrogate takes that loop. Without the check for U+FFFD before the search, every test passes too.Locally, Java 11, with checkstyle, on master
dd4f0f845: janusgraph-driver (130), janusgraph-core (228), janusgraph-inmemory (654, 85 skipped) and janusgraph-test (1,107) pass, and so doesBerkeleyGraphTest(226, 7 skipped). CI runs the graph test on the other storage backends.Notes
Compatibility:
testLongStringsWithoutUnpairedSurrogatesAreWrittenAsBefore).new String(bytes, StandardCharsets.UTF_8)makes of the three bytes, on JDK 11, 17, 21 and 26. A new compressor id would have made such strings unreadable to them: "Unknown compressor type for id".Composite indexes:
SchemaViolationExceptionon the second. Now they have keys of their own.While earlier versions write to the same graph: an earlier version reads such a string written now with U+FFFD in place of each unpaired surrogate, and encodes it so again. So it can't remove the string where its bytes are part of a key:
SETproperty stays, as the earlier version deletes the column of the form it read, which doesn't exist.SET, the unique index finds it by a value it no longer has, and another vertex with that value is rejected with aSchemaViolationException.ConsistencyModifier.LOCKon such a key or index, on a storage backend whose locks check the expected values (CQL, HBase), the earlier version's commit should fail instead with "Expected value mismatch": by code reading, not run.REINDEXadds entries but never removes any, andREMOVE_STALE_ENTRIESremoves only those of elements which no longer exist.Mixed indexes get a string from the transaction as it was written. A reindex, and the CDC worker, read it from storage: so far with question marks in place of its unpaired surrogates, now as written. Each index keeps an unpaired surrogate its own way; Lucene, for one, turns it into U+FFFD.
Order: where JanusGraph sorts strings itself, it now places such a string by its unpaired surrogates, as it places a shorter one, rather than by question marks.
Narrow a transaction's ordered index reads for strings from U+D800 and TEXTSTRING keys #5085 adds its own
REPLACEMENT_CHARACTERand a check for unpaired surrogates toStandardJanusGraphTx. Whichever of the two merges second can share the constant, and a helper where one fits both.Cost, measured with JMH:
StringSerializerwrites and reads a string of 100,000 UTF-16 units, with master's janusgraph-core jar against this branch's on one classpath. Two passes of reversed order ran 3 forks a side each, on Java 11 and an Apple M5 Pro; the table gives the medians of the 6 fork means a side, and an exact Mann-Whitney p over them.getBytes.getBytes, which made the writes of the text with 5% emoji 3.0% slower (p 0.002) in the same benchmark.ED. Searching them byte by byte in one loop made the reads of the string with 10 of them 62% slower than master's in the same benchmark, rather than 25%.Reading the stored bytes: the override of
read(byte[], int, int)alone, measured the same way in an earlier run as the branch with and without it, made every read faster and left writes as they were:🤖 Generated with Claude Code