Skip to content

Commit

Permalink
go/mcap: do not write empty message indexes (#873)
Browse files Browse the repository at this point in the history
### Public-Facing Changes

Go MCAP writer no longer produces message index records for channels not present in a chunk. This helps indexed readers skip chunks that do not contain any messages on a given channel.

### Description
Fixes #862
  • Loading branch information
james-rms authored Mar 30, 2023
1 parent e0297cc commit 741841b
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 9 deletions.
6 changes: 3 additions & 3 deletions go/cli/mcap/cmd/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,9 @@ func TestRecover(t *testing.T) {
assert.Equal(t, 0, attachmentCounter)
assert.Equal(t, 0, metadataCounter)
assert.InDeltaMapValues(t, map[uint16]int{
1: 87,
2: 87,
3: 87,
1: 88,
2: 88,
3: 88,
}, messageCounter, 0.0)
})

Expand Down
4 changes: 4 additions & 0 deletions go/mcap/mcap.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,10 @@ func (idx *MessageIndex) Reset() {
idx.currentIndex = 0
}

func (idx *MessageIndex) IsEmpty() bool {
return idx.currentIndex == 0
}

// Entries lists the entries in the message index.
func (idx *MessageIndex) Entries() []MessageIndexEntry {
return idx.Records[:idx.currentIndex]
Expand Down
2 changes: 1 addition & 1 deletion go/mcap/version.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package mcap

// Version of the MCAP library.
var Version = "v1.0.0"
var Version = "v1.0.1"
3 changes: 2 additions & 1 deletion go/mcap/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,8 @@ func (w *Writer) flushActiveChunk() error {
messageIndexOffsets := make(map[uint16]uint64)
if !w.opts.SkipMessageIndexing {
for _, chanID := range w.channelIDs {
if messageIndex, ok := w.messageIndexes[chanID]; ok {
messageIndex, ok := w.messageIndexes[chanID]
if ok && !messageIndex.IsEmpty() {
messageIndexOffsets[messageIndex.ChannelID] = w.w.Size()
err = w.WriteMessageIndex(messageIndex)
if err != nil {
Expand Down
34 changes: 30 additions & 4 deletions go/mcap/writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ func TestChunkedReadWrite(t *testing.T) {
buf := &bytes.Buffer{}
w, err := NewWriter(buf, &WriterOptions{
Chunked: true,
ChunkSize: 1,
Compression: compression,
IncludeCRC: true,
OverrideLibrary: true,
Expand All @@ -174,6 +175,12 @@ func TestChunkedReadWrite(t *testing.T) {
"callerid": "100", // cspell:disable-line
},
}))
assert.Nil(t, w.WriteChannel(&Channel{
ID: 2,
Topic: "/test2",
MessageEncoding: "ros1",
SchemaID: 1,
}))
assert.Nil(t, w.WriteMessage(&Message{
ChannelID: 1,
Sequence: 0,
Expand All @@ -186,13 +193,25 @@ func TestChunkedReadWrite(t *testing.T) {
4,
},
}))
assert.Nil(t, w.WriteMessage(&Message{
ChannelID: 2,
Sequence: 0,
LogTime: 100,
PublishTime: 100,
Data: []byte{
1,
2,
3,
4,
},
}))
assert.Nil(t, w.Close())
assert.Equal(t, 1, len(w.ChunkIndexes))
assert.Equal(t, 2, len(w.ChunkIndexes))
assert.Equal(t, 0, len(w.AttachmentIndexes))
assert.Equal(t, uint64(1), w.Statistics.MessageCount)
assert.Equal(t, uint64(2), w.Statistics.MessageCount)
assert.Equal(t, uint32(0), w.Statistics.AttachmentCount)
assert.Equal(t, uint32(1), w.Statistics.ChannelCount)
assert.Equal(t, uint32(1), w.Statistics.ChunkCount)
assert.Equal(t, uint32(2), w.Statistics.ChannelCount)
assert.Equal(t, uint32(2), w.Statistics.ChunkCount)
assert.Equal(t, int(w.Offset()), buf.Len())
lexer, err := NewLexer(buf)
assert.Nil(t, err)
Expand All @@ -201,13 +220,20 @@ func TestChunkedReadWrite(t *testing.T) {
TokenHeader,
TokenSchema,
TokenChannel,
TokenChannel,
TokenMessage,
// Note: one message index per chunk, meaning that message indices for channels
// not present in the chunk are not written.
TokenMessageIndex,
TokenMessage,
TokenMessageIndex,
TokenDataEnd,
TokenSchema,
TokenChannel,
TokenChannel,
TokenStatistics,
TokenChunkIndex,
TokenChunkIndex,
TokenSummaryOffset,
TokenSummaryOffset,
TokenSummaryOffset,
Expand Down

0 comments on commit 741841b

Please sign in to comment.