From 0de36e290746b6af0831d6fd6444461104ed2a95 Mon Sep 17 00:00:00 2001 From: Radu Berinde Date: Mon, 30 Sep 2019 17:00:04 -0400 Subject: [PATCH 1/2] sql: move histogram decoding into the stats cache Currently the stats cache exposes the encoded histogram data, and the opt catalog decodes it. The problem is that opt catalog objects are not shared across connections so this is inefficient (especially in memory usage). This change moves the decoding into the stats cache. The opt catalog objects now simply point to the data computed by the cache. There are still inefficiencies to improve on in the future: the opt catalog can hold on to multiple versions of tables, so we will keep many versions of histograms "alive". Informs #41206. Informs #40922. Release justification: fix for new functionality. Release note: None --- pkg/ccl/backupccl/backup.go | 6 +- pkg/ccl/backupccl/backup.pb.go | 152 ++++++++++++++-------------- pkg/ccl/backupccl/backup.proto | 2 +- pkg/ccl/backupccl/restore.go | 6 +- pkg/sql/opt_catalog.go | 43 ++------ pkg/sql/stats/delete_stats_test.go | 2 +- pkg/sql/stats/new_stat.go | 4 +- pkg/sql/stats/stats_cache.go | 57 ++++++++--- pkg/sql/stats/stats_cache_test.go | 35 +++++-- pkg/sql/stats/table_statistic.pb.go | 134 ++++++++++++------------ pkg/sql/stats/table_statistic.proto | 12 +-- 11 files changed, 237 insertions(+), 216 deletions(-) diff --git a/pkg/ccl/backupccl/backup.go b/pkg/ccl/backupccl/backup.go index c369bac43ddc..bfade93d1d8b 100644 --- a/pkg/ccl/backupccl/backup.go +++ b/pkg/ccl/backupccl/backup.go @@ -1045,7 +1045,7 @@ func backupPlanHook( } statsCache := p.ExecCfg().TableStatsCache - tableStatistics := make([]*stats.TableStatistic, 0) + tableStatistics := make([]*stats.TableStatisticProto, 0) var tables []*sqlbase.TableDescriptor for _, desc := range targetDescs { if dbDesc := desc.GetDatabase(); dbDesc != nil { @@ -1064,7 +1064,9 @@ func backupPlanHook( if err != nil { return err } - tableStatistics = append(tableStatistics, tableStatisticsAcc...) + for i := range tableStatisticsAcc { + tableStatistics = append(tableStatistics, &tableStatisticsAcc[i].TableStatisticProto) + } } } diff --git a/pkg/ccl/backupccl/backup.pb.go b/pkg/ccl/backupccl/backup.pb.go index c0107212b969..991e09918751 100644 --- a/pkg/ccl/backupccl/backup.pb.go +++ b/pkg/ccl/backupccl/backup.pb.go @@ -49,7 +49,7 @@ func (x MVCCFilter) String() string { return proto.EnumName(MVCCFilter_name, int32(x)) } func (MVCCFilter) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_backup_b369a1a0dd1a82de, []int{0} + return fileDescriptor_backup_d569378b1db001a7, []int{0} } // BackupDescriptor represents a consistent snapshot of ranges. @@ -92,14 +92,14 @@ type BackupDescriptor struct { ID github_com_cockroachdb_cockroach_pkg_util_uuid.UUID `protobuf:"bytes,18,opt,name=id,proto3,customtype=github.com/cockroachdb/cockroach/pkg/util/uuid.UUID" json:"id"` PartitionDescriptorFilenames []string `protobuf:"bytes,19,rep,name=partition_descriptor_filenames,json=partitionDescriptorFilenames,proto3" json:"partition_descriptor_filenames,omitempty"` LocalityKVs []string `protobuf:"bytes,20,rep,name=locality_kvs,json=localityKvs,proto3" json:"locality_kvs,omitempty"` - Statistics []*stats.TableStatistic `protobuf:"bytes,21,rep,name=statistics,proto3" json:"statistics,omitempty"` + Statistics []*stats.TableStatisticProto `protobuf:"bytes,21,rep,name=statistics,proto3" json:"statistics,omitempty"` } func (m *BackupDescriptor) Reset() { *m = BackupDescriptor{} } func (m *BackupDescriptor) String() string { return proto.CompactTextString(m) } func (*BackupDescriptor) ProtoMessage() {} func (*BackupDescriptor) Descriptor() ([]byte, []int) { - return fileDescriptor_backup_b369a1a0dd1a82de, []int{0} + return fileDescriptor_backup_d569378b1db001a7, []int{0} } func (m *BackupDescriptor) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -142,7 +142,7 @@ func (m *BackupDescriptor_File) Reset() { *m = BackupDescriptor_File{} } func (m *BackupDescriptor_File) String() string { return proto.CompactTextString(m) } func (*BackupDescriptor_File) ProtoMessage() {} func (*BackupDescriptor_File) Descriptor() ([]byte, []int) { - return fileDescriptor_backup_b369a1a0dd1a82de, []int{0, 0} + return fileDescriptor_backup_d569378b1db001a7, []int{0, 0} } func (m *BackupDescriptor_File) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -177,7 +177,7 @@ func (m *BackupDescriptor_DescriptorRevision) Reset() { *m = BackupDescr func (m *BackupDescriptor_DescriptorRevision) String() string { return proto.CompactTextString(m) } func (*BackupDescriptor_DescriptorRevision) ProtoMessage() {} func (*BackupDescriptor_DescriptorRevision) Descriptor() ([]byte, []int) { - return fileDescriptor_backup_b369a1a0dd1a82de, []int{0, 1} + return fileDescriptor_backup_d569378b1db001a7, []int{0, 1} } func (m *BackupDescriptor_DescriptorRevision) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -212,7 +212,7 @@ func (m *BackupPartitionDescriptor) Reset() { *m = BackupPartitionDescri func (m *BackupPartitionDescriptor) String() string { return proto.CompactTextString(m) } func (*BackupPartitionDescriptor) ProtoMessage() {} func (*BackupPartitionDescriptor) Descriptor() ([]byte, []int) { - return fileDescriptor_backup_b369a1a0dd1a82de, []int{1} + return fileDescriptor_backup_d569378b1db001a7, []int{1} } func (m *BackupPartitionDescriptor) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -1442,7 +1442,7 @@ func (m *BackupDescriptor) Unmarshal(dAtA []byte) error { if postIndex > l { return io.ErrUnexpectedEOF } - m.Statistics = append(m.Statistics, &stats.TableStatistic{}) + m.Statistics = append(m.Statistics, &stats.TableStatisticProto{}) if err := m.Statistics[len(m.Statistics)-1].Unmarshal(dAtA[iNdEx:postIndex]); err != nil { return err } @@ -2104,75 +2104,75 @@ var ( ErrIntOverflowBackup = fmt.Errorf("proto: integer overflow") ) -func init() { proto.RegisterFile("ccl/backupccl/backup.proto", fileDescriptor_backup_b369a1a0dd1a82de) } +func init() { proto.RegisterFile("ccl/backupccl/backup.proto", fileDescriptor_backup_d569378b1db001a7) } -var fileDescriptor_backup_b369a1a0dd1a82de = []byte{ +var fileDescriptor_backup_d569378b1db001a7 = []byte{ // 1067 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xa4, 0x56, 0xcd, 0x6e, 0xdb, 0x46, - 0x10, 0x16, 0x25, 0x5a, 0x3f, 0x2b, 0xff, 0xc8, 0x9b, 0xa4, 0x61, 0x85, 0x54, 0x52, 0x12, 0x14, - 0x10, 0x5a, 0x80, 0x44, 0x6c, 0x04, 0x29, 0x8a, 0xa2, 0x80, 0x25, 0xd6, 0x09, 0x9d, 0xf4, 0x8f, - 0x72, 0x7c, 0xf0, 0x85, 0x58, 0x2d, 0xd7, 0xd2, 0xc2, 0x14, 0x49, 0x73, 0x97, 0x46, 0xfd, 0x16, - 0x7d, 0x93, 0xbe, 0x86, 0x2f, 0x05, 0x72, 0x0c, 0x7a, 0x10, 0x5a, 0xf9, 0x09, 0x7a, 0xed, 0xa1, - 0x28, 0x76, 0x49, 0x8a, 0xaa, 0x5d, 0xc3, 0x6a, 0x74, 0x1b, 0x0d, 0xe7, 0xfb, 0x76, 0x77, 0x66, - 0xbe, 0x19, 0x81, 0x26, 0xc6, 0x9e, 0x31, 0x44, 0xf8, 0x34, 0x0e, 0x73, 0x4b, 0x0f, 0xa3, 0x80, - 0x07, 0xf0, 0x21, 0x0e, 0xf0, 0x69, 0x14, 0x20, 0x3c, 0xd6, 0x31, 0xf6, 0xf4, 0x79, 0x54, 0xb3, - 0x31, 0x8c, 0xa9, 0xe7, 0x1a, 0xd4, 0x3f, 0x09, 0x92, 0xd0, 0xe6, 0xb6, 0x0c, 0x0b, 0x87, 0x06, - 0x0a, 0x69, 0xea, 0x82, 0x99, 0xcb, 0x45, 0x1c, 0xa5, 0xbe, 0x36, 0x3b, 0xf3, 0x0c, 0xc6, 0x11, - 0x67, 0x06, 0x47, 0x43, 0x8f, 0x38, 0xc2, 0xa6, 0x8c, 0x53, 0x9c, 0x06, 0x3c, 0x92, 0x01, 0x67, - 0xde, 0x10, 0x31, 0x62, 0x30, 0x1e, 0xc5, 0x98, 0xc7, 0x11, 0x71, 0xd3, 0xaf, 0x5a, 0xcc, 0xa9, - 0x67, 0x8c, 0x3d, 0x6c, 0x70, 0x3a, 0x21, 0x8c, 0xa3, 0x49, 0x7a, 0xd5, 0xe6, 0xfd, 0x51, 0x30, - 0x0a, 0xa4, 0x69, 0x08, 0x2b, 0xf1, 0x3e, 0xf9, 0xa5, 0x01, 0x1a, 0x3d, 0x79, 0x6b, 0x93, 0x30, - 0x1c, 0xd1, 0x90, 0x07, 0x11, 0xec, 0x01, 0xc0, 0x38, 0x8a, 0xb8, 0x23, 0x38, 0x34, 0xa5, 0xa3, - 0x74, 0xeb, 0x3b, 0x9f, 0xe8, 0xf9, 0x53, 0xc5, 0x19, 0xfa, 0xd8, 0xc3, 0xfa, 0x61, 0x76, 0x46, - 0x4f, 0xbd, 0x9c, 0xb6, 0x0b, 0x76, 0x4d, 0xc2, 0x84, 0x17, 0x7e, 0x0d, 0xaa, 0xc4, 0x77, 0x13, - 0x86, 0xe2, 0xf2, 0x0c, 0x15, 0xe2, 0xbb, 0x12, 0xbf, 0x0b, 0xd6, 0x58, 0x88, 0x7c, 0xa6, 0x95, - 0x3a, 0xa5, 0x6e, 0x7d, 0xe7, 0xe1, 0x02, 0x38, 0xcd, 0x9a, 0x3e, 0x08, 0x91, 0x9f, 0xc2, 0x92, - 0x58, 0x78, 0x00, 0xd6, 0x4e, 0xa8, 0x47, 0x98, 0xa6, 0x4a, 0x90, 0xae, 0xdf, 0x52, 0x1e, 0xfd, - 0xfa, 0x93, 0xf5, 0x7d, 0xea, 0x91, 0x8c, 0x4b, 0x52, 0x40, 0x0b, 0xd4, 0xdd, 0xf9, 0x77, 0xa6, - 0xad, 0x49, 0xc6, 0xc7, 0x0b, 0x8c, 0xec, 0xcc, 0xd3, 0xd3, 0x3a, 0xe8, 0x39, 0x53, 0x4a, 0xb2, - 0x88, 0x85, 0x5f, 0x80, 0x92, 0x4b, 0x23, 0xad, 0x22, 0xd3, 0xd0, 0xf9, 0x8f, 0x97, 0x7c, 0xf3, - 0x53, 0x18, 0x44, 0x7c, 0xc0, 0x83, 0x08, 0x8d, 0xb2, 0x6b, 0x08, 0x08, 0xfc, 0x14, 0x6c, 0x9e, - 0x04, 0xd1, 0x04, 0x71, 0xe7, 0x9c, 0x44, 0x8c, 0x06, 0xbe, 0x56, 0xed, 0x28, 0xdd, 0x0d, 0x7b, - 0x23, 0xf1, 0x1e, 0x25, 0x4e, 0x38, 0x02, 0x00, 0x7b, 0x31, 0xe3, 0x24, 0x72, 0xa8, 0xab, 0xd5, - 0x3a, 0x4a, 0x77, 0xbd, 0xf7, 0x4a, 0xb0, 0xfc, 0x36, 0x6d, 0xef, 0x8e, 0x28, 0x1f, 0xc7, 0x43, - 0x1d, 0x07, 0x13, 0x63, 0x7e, 0xb2, 0x3b, 0xcc, 0x6d, 0x23, 0x3c, 0x1d, 0x19, 0xb2, 0x6d, 0xe2, - 0x98, 0xba, 0xfa, 0xdb, 0xb7, 0x96, 0x39, 0x9b, 0xb6, 0x6b, 0xfd, 0x84, 0xd0, 0x32, 0xed, 0x5a, - 0xca, 0x6d, 0xb9, 0xf0, 0x18, 0x54, 0xfc, 0xc0, 0x25, 0xe2, 0x14, 0xd0, 0x51, 0xba, 0x6b, 0xbd, - 0xbd, 0xd9, 0xb4, 0x5d, 0xfe, 0x2e, 0x70, 0x89, 0x65, 0xfe, 0xb5, 0xec, 0x59, 0xd9, 0xab, 0x13, - 0x98, 0x5d, 0x16, 0x8c, 0x96, 0x0b, 0xbf, 0x04, 0x40, 0x8a, 0xc6, 0x11, 0xa2, 0xd1, 0xea, 0x32, - 0x59, 0x0f, 0x16, 0x92, 0x25, 0x3f, 0xea, 0x96, 0x7f, 0x12, 0x64, 0xdd, 0x26, 0x3d, 0xc2, 0x01, - 0x2d, 0xb0, 0x4e, 0x7c, 0x1e, 0x5d, 0x38, 0x38, 0x88, 0x7d, 0xce, 0xb4, 0xf5, 0x5b, 0x53, 0xdd, - 0x8b, 0xbd, 0xd3, 0xef, 0xc3, 0x41, 0x3c, 0x99, 0xa0, 0xe8, 0x22, 0x2b, 0x96, 0xc4, 0xf6, 0x25, - 0x14, 0x1e, 0x82, 0xfa, 0xe4, 0x1c, 0x63, 0xe7, 0x84, 0x7a, 0x9c, 0x44, 0xda, 0x46, 0x47, 0xe9, - 0x6e, 0xee, 0x3c, 0xbd, 0xb5, 0x93, 0xbe, 0x3d, 0xea, 0xf7, 0xf7, 0x65, 0x68, 0x6f, 0x73, 0x36, - 0x6d, 0x83, 0xfc, 0xb7, 0x0d, 0x04, 0x4f, 0x62, 0xc3, 0x63, 0xb0, 0x8e, 0x83, 0x49, 0xe8, 0x11, - 0x4e, 0x1c, 0x77, 0xc8, 0xb4, 0xcd, 0x4e, 0xa9, 0xbb, 0xd1, 0x7b, 0xb1, 0x74, 0xce, 0x16, 0x44, - 0xaf, 0x5b, 0xa6, 0x5d, 0xcf, 0xc8, 0xcc, 0x21, 0x83, 0xaf, 0x40, 0x83, 0xfa, 0x3c, 0x0a, 0xdc, - 0x18, 0x13, 0xd7, 0x49, 0x54, 0xb3, 0xb5, 0x8c, 0x6a, 0xb6, 0x72, 0xd8, 0x40, 0xea, 0xe7, 0x0c, - 0xc0, 0xbc, 0x6f, 0x1d, 0x3c, 0x46, 0xfe, 0x88, 0x30, 0xad, 0x21, 0xb9, 0xbe, 0x5a, 0x5e, 0x4c, - 0xb9, 0x69, 0x93, 0x73, 0x2a, 0x3a, 0x34, 0x3d, 0x70, 0x3b, 0x67, 0xef, 0x27, 0xe4, 0x70, 0x00, - 0xee, 0x45, 0x69, 0x90, 0xb3, 0x30, 0x74, 0xb6, 0x97, 0x1f, 0x19, 0xdb, 0x19, 0x7e, 0x30, 0x1f, - 0x3e, 0x3f, 0x82, 0x22, 0x75, 0x35, 0x28, 0x75, 0xb0, 0xb7, 0x9a, 0x0e, 0x8a, 0x96, 0x69, 0x17, - 0xa9, 0x0b, 0x4d, 0xd0, 0x0a, 0x51, 0xc4, 0x29, 0x17, 0x17, 0x5d, 0x48, 0x92, 0x18, 0x16, 0x3e, - 0x9a, 0x10, 0xa6, 0xdd, 0xeb, 0x94, 0xba, 0x35, 0xfb, 0xd1, 0x3c, 0x2a, 0xcf, 0xc2, 0x7e, 0x16, - 0x03, 0x77, 0xc0, 0xba, 0x17, 0x60, 0xe4, 0x51, 0x7e, 0xe1, 0x9c, 0x9e, 0x33, 0xed, 0xbe, 0xc0, - 0xf4, 0xb6, 0x66, 0xd3, 0x76, 0xfd, 0x4d, 0xea, 0x7f, 0x7d, 0xc4, 0xec, 0x7a, 0x16, 0xf4, 0xfa, - 0x9c, 0xc1, 0xbe, 0x9c, 0xc6, 0xc9, 0x0e, 0x60, 0xda, 0x03, 0x59, 0x8c, 0xa7, 0xd7, 0xe7, 0x90, - 0x58, 0x18, 0xfa, 0xa1, 0x58, 0x18, 0x83, 0x2c, 0xd6, 0x5e, 0x80, 0x35, 0xff, 0x2c, 0x02, 0x55, - 0x5c, 0x03, 0x3e, 0x03, 0xaa, 0xe8, 0x90, 0x74, 0xaa, 0xdf, 0xd1, 0x20, 0x32, 0x14, 0x42, 0xa0, - 0x86, 0x88, 0x8f, 0xe5, 0x18, 0xaf, 0xd9, 0xd2, 0x86, 0x1f, 0x81, 0x32, 0x1b, 0xa3, 0xe7, 0xcf, - 0x76, 0x34, 0x55, 0x64, 0xd9, 0x4e, 0x7f, 0xdd, 0x10, 0x62, 0xf9, 0xc3, 0x85, 0xf8, 0xef, 0x2d, - 0x54, 0x59, 0x79, 0x0b, 0x55, 0x3f, 0x60, 0x0b, 0x19, 0xa0, 0xbe, 0x50, 0x2f, 0x39, 0x59, 0x6b, - 0x89, 0xce, 0xf3, 0x72, 0xd9, 0x20, 0xaf, 0xd6, 0x81, 0x5a, 0x2d, 0x35, 0xd4, 0x03, 0xb5, 0xba, - 0xd6, 0x28, 0x37, 0x7f, 0x55, 0x00, 0xbc, 0x29, 0x05, 0xf8, 0x02, 0xa8, 0xff, 0x77, 0xaf, 0x4a, - 0x00, 0x7c, 0x09, 0x8a, 0x96, 0x29, 0xab, 0xb0, 0xc2, 0xe4, 0x28, 0x5a, 0x26, 0x7c, 0x0e, 0x54, - 0xd1, 0xc1, 0x5a, 0x49, 0xde, 0xe0, 0xee, 0x9d, 0x66, 0xcb, 0xf0, 0x03, 0xb5, 0x5a, 0x6e, 0x54, - 0x9e, 0xfc, 0xad, 0x80, 0x8f, 0x13, 0xc5, 0xff, 0x70, 0xb3, 0xd3, 0xaf, 0x27, 0x4c, 0xb9, 0x2b, - 0x61, 0xf9, 0xca, 0x2e, 0xae, 0xbe, 0xb2, 0x5d, 0x50, 0x4b, 0xe2, 0xc5, 0x7e, 0x2a, 0x49, 0xf5, - 0xbf, 0x5c, 0x4d, 0xfd, 0xd5, 0xe4, 0x54, 0xcb, 0xb4, 0xab, 0x09, 0xb3, 0xe5, 0x7e, 0xf6, 0x18, - 0x2c, 0x0c, 0x79, 0x08, 0x40, 0xf9, 0x0d, 0xe2, 0x84, 0xf1, 0x46, 0x01, 0x56, 0x40, 0x69, 0xcf, - 0xf3, 0x1a, 0x4a, 0xef, 0xf3, 0xcb, 0x3f, 0x5a, 0x85, 0xcb, 0x59, 0x4b, 0x79, 0x37, 0x6b, 0x29, - 0xef, 0x67, 0x2d, 0xe5, 0xf7, 0x59, 0x4b, 0xf9, 0xf9, 0xaa, 0x55, 0x78, 0x77, 0xd5, 0x2a, 0xbc, - 0xbf, 0x6a, 0x15, 0x8e, 0x6b, 0xf3, 0x87, 0x0d, 0xcb, 0xf2, 0x9f, 0xd8, 0xee, 0x3f, 0x01, 0x00, - 0x00, 0xff, 0xff, 0x40, 0x5c, 0x94, 0xd7, 0x68, 0x0a, 0x00, 0x00, + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xa4, 0x56, 0xcb, 0x6e, 0xdb, 0x46, + 0x14, 0x15, 0x25, 0x5a, 0x8f, 0x2b, 0x3f, 0xe4, 0x49, 0xd2, 0xb0, 0x42, 0x2a, 0x29, 0x29, 0x0a, + 0x08, 0x2d, 0x40, 0x22, 0x36, 0x82, 0x14, 0x45, 0x51, 0xc0, 0xb2, 0xea, 0x84, 0x4e, 0xda, 0xa6, + 0x94, 0xe3, 0x85, 0x37, 0x04, 0x35, 0x1c, 0x4b, 0x03, 0x53, 0x24, 0xcd, 0x19, 0x1a, 0xf5, 0x5f, + 0xf4, 0x63, 0xfa, 0x11, 0xde, 0x14, 0xc8, 0x32, 0xe8, 0x42, 0x68, 0xe5, 0x2f, 0xe8, 0xb6, 0x8b, + 0xa2, 0x98, 0x21, 0x29, 0xaa, 0x76, 0x0d, 0xab, 0xf1, 0xee, 0xea, 0xf2, 0x9c, 0x33, 0x33, 0xf7, + 0x29, 0x68, 0x62, 0xec, 0x19, 0x43, 0x07, 0x9f, 0xc4, 0x61, 0x6e, 0xe9, 0x61, 0x14, 0xf0, 0x00, + 0x3d, 0xc4, 0x01, 0x3e, 0x89, 0x02, 0x07, 0x8f, 0x75, 0x8c, 0x3d, 0x7d, 0x8e, 0x6a, 0x36, 0x86, + 0x31, 0xf5, 0x5c, 0x83, 0xfa, 0xc7, 0x41, 0x02, 0x6d, 0x6e, 0x4a, 0x58, 0x38, 0x34, 0x9c, 0x90, + 0xa6, 0x2e, 0x94, 0xb9, 0x5c, 0x87, 0x3b, 0xa9, 0xaf, 0xcd, 0x4e, 0x3d, 0x83, 0x71, 0x87, 0x33, + 0x83, 0x3b, 0x43, 0x8f, 0xd8, 0xc2, 0xa6, 0x8c, 0x53, 0x9c, 0x02, 0x1e, 0x49, 0xc0, 0xa9, 0x37, + 0x74, 0x18, 0x31, 0x18, 0x8f, 0x62, 0xcc, 0xe3, 0x88, 0xb8, 0xe9, 0x57, 0x2d, 0xe6, 0xd4, 0x33, + 0xc6, 0x1e, 0x36, 0x38, 0x9d, 0x10, 0xc6, 0x9d, 0x49, 0x7a, 0xd5, 0xe6, 0xfd, 0x51, 0x30, 0x0a, + 0xa4, 0x69, 0x08, 0x2b, 0xf1, 0x3e, 0xf9, 0xa5, 0x01, 0x8d, 0x9e, 0xbc, 0x75, 0x9f, 0x30, 0x1c, + 0xd1, 0x90, 0x07, 0x11, 0xea, 0x01, 0x30, 0xee, 0x44, 0xdc, 0x16, 0x1a, 0x9a, 0xd2, 0x51, 0xba, + 0xf5, 0xad, 0x4f, 0xf4, 0xfc, 0xa9, 0xe2, 0x0c, 0x7d, 0xec, 0x61, 0xfd, 0x20, 0x3b, 0xa3, 0xa7, + 0x5e, 0x4c, 0xdb, 0x05, 0xab, 0x26, 0x69, 0xc2, 0x8b, 0xbe, 0x81, 0x2a, 0xf1, 0xdd, 0x44, 0xa1, + 0xb8, 0xbc, 0x42, 0x85, 0xf8, 0xae, 0xe4, 0x6f, 0xc3, 0x0a, 0x0b, 0x1d, 0x9f, 0x69, 0xa5, 0x4e, + 0xa9, 0x5b, 0xdf, 0x7a, 0xb8, 0x40, 0x4e, 0xa3, 0xa6, 0x0f, 0x42, 0xc7, 0x4f, 0x69, 0x09, 0x16, + 0xed, 0xc3, 0xca, 0x31, 0xf5, 0x08, 0xd3, 0x54, 0x49, 0xd2, 0xf5, 0x1b, 0xd2, 0xa3, 0x5f, 0x7d, + 0xb2, 0xbe, 0x47, 0x3d, 0x92, 0x69, 0x49, 0x09, 0x64, 0x42, 0xdd, 0x9d, 0x7f, 0x67, 0xda, 0x8a, + 0x54, 0x7c, 0xbc, 0xa0, 0xc8, 0x4e, 0x3d, 0x3d, 0xcd, 0x83, 0x9e, 0x2b, 0xa5, 0x22, 0x8b, 0x5c, + 0xf4, 0x25, 0x94, 0x5c, 0x1a, 0x69, 0x15, 0x19, 0x86, 0xce, 0x7f, 0xbc, 0xe4, 0xdb, 0x9f, 0xc2, + 0x20, 0xe2, 0x03, 0x1e, 0x44, 0xce, 0x28, 0xbb, 0x86, 0xa0, 0xa0, 0xcf, 0x60, 0xfd, 0x38, 0x88, + 0x26, 0x0e, 0xb7, 0xcf, 0x48, 0xc4, 0x68, 0xe0, 0x6b, 0xd5, 0x8e, 0xd2, 0x5d, 0xb3, 0xd6, 0x12, + 0xef, 0x61, 0xe2, 0x44, 0x23, 0x00, 0xec, 0xc5, 0x8c, 0x93, 0xc8, 0xa6, 0xae, 0x56, 0xeb, 0x28, + 0xdd, 0xd5, 0xde, 0x4b, 0xa1, 0xf2, 0xdb, 0xb4, 0xbd, 0x3d, 0xa2, 0x7c, 0x1c, 0x0f, 0x75, 0x1c, + 0x4c, 0x8c, 0xf9, 0xc9, 0xee, 0x30, 0xb7, 0x8d, 0xf0, 0x64, 0x64, 0xc8, 0xb2, 0x89, 0x63, 0xea, + 0xea, 0x6f, 0xdf, 0x9a, 0xfd, 0xd9, 0xb4, 0x5d, 0xdb, 0x4d, 0x04, 0xcd, 0xbe, 0x55, 0x4b, 0xb5, + 0x4d, 0x17, 0x1d, 0x41, 0xc5, 0x0f, 0x5c, 0x22, 0x4e, 0x81, 0x8e, 0xd2, 0x5d, 0xe9, 0xed, 0xcc, + 0xa6, 0xed, 0xf2, 0xf7, 0x81, 0x4b, 0xcc, 0xfe, 0x5f, 0xcb, 0x9e, 0x95, 0xbd, 0x3a, 0xa1, 0x59, + 0x65, 0xa1, 0x68, 0xba, 0xe8, 0x2b, 0x00, 0xd9, 0x34, 0xb6, 0x68, 0x1a, 0xad, 0x2e, 0x83, 0xf5, + 0x60, 0x21, 0x58, 0xf2, 0xa3, 0x6e, 0xfa, 0xc7, 0x41, 0x56, 0x6d, 0xd2, 0x23, 0x1c, 0xc8, 0x84, + 0x55, 0xe2, 0xf3, 0xe8, 0xdc, 0xc6, 0x41, 0xec, 0x73, 0xa6, 0xad, 0xde, 0x18, 0xea, 0x5e, 0xec, + 0x9d, 0xfc, 0x10, 0x0e, 0xe2, 0xc9, 0xc4, 0x89, 0xce, 0xb3, 0x64, 0x49, 0xee, 0xae, 0xa4, 0xa2, + 0x03, 0xa8, 0x4f, 0xce, 0x30, 0xb6, 0x8f, 0xa9, 0xc7, 0x49, 0xa4, 0xad, 0x75, 0x94, 0xee, 0xfa, + 0xd6, 0xa7, 0x37, 0x56, 0xd2, 0x77, 0x87, 0xbb, 0xbb, 0x7b, 0x12, 0xda, 0x5b, 0x9f, 0x4d, 0xdb, + 0x90, 0xff, 0xb6, 0x40, 0xe8, 0x24, 0x36, 0x3a, 0x82, 0x55, 0x1c, 0x4c, 0x42, 0x8f, 0x70, 0x62, + 0xbb, 0x43, 0xa6, 0xad, 0x77, 0x4a, 0xdd, 0xb5, 0xde, 0xf3, 0xa5, 0x63, 0xb6, 0xd0, 0xf4, 0xba, + 0xd9, 0xb7, 0xea, 0x99, 0x58, 0x7f, 0xc8, 0xd0, 0x4b, 0x68, 0x50, 0x9f, 0x47, 0x81, 0x1b, 0x63, + 0xe2, 0xda, 0x49, 0xd7, 0x6c, 0x2c, 0xd3, 0x35, 0x1b, 0x39, 0x6d, 0x20, 0xfb, 0xe7, 0x14, 0x50, + 0x5e, 0xb7, 0x36, 0x1e, 0x3b, 0xfe, 0x88, 0x30, 0xad, 0x21, 0xb5, 0xbe, 0x5e, 0xbe, 0x99, 0x72, + 0xd3, 0x22, 0x67, 0x54, 0x54, 0x68, 0x7a, 0xe0, 0x66, 0xae, 0xbe, 0x9b, 0x88, 0xa3, 0x01, 0xdc, + 0x8b, 0x52, 0x90, 0xbd, 0x30, 0x74, 0x36, 0x97, 0x1f, 0x19, 0x9b, 0x19, 0x7f, 0x30, 0x1f, 0x3e, + 0x3f, 0x42, 0x91, 0xba, 0x1a, 0x92, 0x7d, 0xb0, 0x73, 0xb7, 0x3e, 0x28, 0x9a, 0x7d, 0xab, 0x48, + 0x5d, 0xd4, 0x87, 0x56, 0xe8, 0x44, 0x9c, 0x72, 0x71, 0xd1, 0x85, 0x20, 0x89, 0x61, 0xe1, 0x3b, + 0x13, 0xc2, 0xb4, 0x7b, 0x9d, 0x52, 0xb7, 0x66, 0x3d, 0x9a, 0xa3, 0xf2, 0x28, 0xec, 0x65, 0x18, + 0xb4, 0x05, 0xab, 0x5e, 0x80, 0x1d, 0x8f, 0xf2, 0x73, 0xfb, 0xe4, 0x8c, 0x69, 0xf7, 0x05, 0xa7, + 0xb7, 0x31, 0x9b, 0xb6, 0xeb, 0xaf, 0x53, 0xff, 0xab, 0x43, 0x66, 0xd5, 0x33, 0xd0, 0xab, 0x33, + 0x91, 0x5e, 0x98, 0xef, 0x00, 0xa6, 0x3d, 0x90, 0xc9, 0xe8, 0x5e, 0x9d, 0x43, 0x62, 0x61, 0xe8, + 0x07, 0x62, 0x61, 0x0c, 0x32, 0xec, 0x1b, 0x31, 0xe0, 0xad, 0x05, 0x6e, 0xf3, 0xcf, 0x22, 0xa8, + 0xe2, 0x2e, 0xe8, 0x29, 0xa8, 0xa2, 0x4c, 0xd2, 0xd1, 0x7e, 0x4b, 0x95, 0x48, 0x28, 0x42, 0xa0, + 0x86, 0x0e, 0x1f, 0xcb, 0x59, 0x5e, 0xb3, 0xa4, 0x8d, 0x3e, 0x82, 0x32, 0x1b, 0x3b, 0xcf, 0x9e, + 0x6e, 0x69, 0xaa, 0x08, 0xb5, 0x95, 0xfe, 0xba, 0xd6, 0x8d, 0xe5, 0x0f, 0xef, 0xc6, 0x7f, 0xaf, + 0xa2, 0xca, 0x9d, 0x57, 0x51, 0xf5, 0x03, 0x56, 0x91, 0x01, 0xf5, 0x85, 0xa4, 0xc9, 0xf1, 0x5a, + 0x4b, 0x9a, 0x3d, 0xcf, 0x99, 0x05, 0x79, 0xca, 0xf6, 0xd5, 0x6a, 0xa9, 0xa1, 0xee, 0xab, 0xd5, + 0x95, 0x46, 0xb9, 0xf9, 0xab, 0x02, 0xe8, 0x7a, 0x3f, 0xa0, 0xe7, 0xa0, 0xfe, 0xdf, 0xe5, 0x2a, + 0x09, 0xe8, 0x05, 0x14, 0xcd, 0xbe, 0xcc, 0xc2, 0x1d, 0xc6, 0x47, 0xd1, 0xec, 0xa3, 0x67, 0xa0, + 0x8a, 0x32, 0xd6, 0x4a, 0xf2, 0x06, 0xb7, 0x2f, 0x36, 0x4b, 0xc2, 0xf7, 0xd5, 0x6a, 0xb9, 0x51, + 0x79, 0xf2, 0xb7, 0x02, 0x1f, 0x27, 0x6d, 0xff, 0xe6, 0x7a, 0xb9, 0x5f, 0x0d, 0x98, 0x72, 0x5b, + 0xc0, 0xf2, 0xbd, 0x5d, 0xbc, 0xfb, 0xde, 0x76, 0xa1, 0x96, 0xe0, 0xc5, 0x92, 0x2a, 0xc9, 0x11, + 0xf0, 0xe2, 0x6e, 0x23, 0xa0, 0x9a, 0x9c, 0x6a, 0xf6, 0xad, 0x6a, 0xa2, 0x6c, 0xba, 0x9f, 0x3f, + 0x86, 0x85, 0x49, 0x8f, 0x00, 0xca, 0xaf, 0x1d, 0x4e, 0x18, 0x6f, 0x14, 0x50, 0x05, 0x4a, 0x3b, + 0x9e, 0xd7, 0x50, 0x7a, 0x5f, 0x5c, 0xfc, 0xd1, 0x2a, 0x5c, 0xcc, 0x5a, 0xca, 0xbb, 0x59, 0x4b, + 0x79, 0x3f, 0x6b, 0x29, 0xbf, 0xcf, 0x5a, 0xca, 0xcf, 0x97, 0xad, 0xc2, 0xbb, 0xcb, 0x56, 0xe1, + 0xfd, 0x65, 0xab, 0x70, 0x54, 0x9b, 0x3f, 0x6c, 0x58, 0x96, 0x7f, 0xc7, 0xb6, 0xff, 0x09, 0x00, + 0x00, 0xff, 0xff, 0xbd, 0xb1, 0x12, 0x8d, 0x6d, 0x0a, 0x00, 0x00, } diff --git a/pkg/ccl/backupccl/backup.proto b/pkg/ccl/backupccl/backup.proto index f8c7bcfb04e5..eb26dfc3b589 100644 --- a/pkg/ccl/backupccl/backup.proto +++ b/pkg/ccl/backupccl/backup.proto @@ -101,7 +101,7 @@ message BackupDescriptor { (gogoproto.customtype) = "github.com/cockroachdb/cockroach/pkg/util/uuid.UUID"]; repeated string partition_descriptor_filenames = 19; repeated string locality_kvs = 20 [(gogoproto.customname) = "LocalityKVs"]; - repeated sql.stats.TableStatistic statistics = 21; + repeated sql.stats.TableStatisticProto statistics = 21; } message BackupPartitionDescriptor{ diff --git a/pkg/ccl/backupccl/restore.go b/pkg/ccl/backupccl/restore.go index b97da2044df9..a73726a28f59 100644 --- a/pkg/ccl/backupccl/restore.go +++ b/pkg/ccl/backupccl/restore.go @@ -1644,7 +1644,7 @@ type restoreResumer struct { databases []*sqlbase.DatabaseDescriptor tables []*sqlbase.TableDescriptor exec sqlutil.InternalExecutor - latestStats []*stats.TableStatistic + latestStats []*stats.TableStatisticProto statsRefresher *stats.Refresher } @@ -1656,8 +1656,8 @@ type restoreResumer struct { // table is being restored. func remapRelevantStatistics( backup BackupDescriptor, tableRewrites TableRewriteMap, -) []*stats.TableStatistic { - relevantTableStatistics := make([]*stats.TableStatistic, 0, len(backup.Statistics)) +) []*stats.TableStatisticProto { + relevantTableStatistics := make([]*stats.TableStatisticProto, 0, len(backup.Statistics)) for i := range backup.Statistics { stat := backup.Statistics[i] diff --git a/pkg/sql/opt_catalog.go b/pkg/sql/opt_catalog.go index d4cc982716eb..12f8a1040a2c 100644 --- a/pkg/sql/opt_catalog.go +++ b/pkg/sql/opt_catalog.go @@ -26,7 +26,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" "github.com/cockroachdb/cockroach/pkg/sql/stats" "github.com/cockroachdb/cockroach/pkg/util" - "github.com/cockroachdb/cockroach/pkg/util/encoding" "github.com/cockroachdb/errors" ) @@ -1001,22 +1000,14 @@ func (oi *optIndex) PartitionByListPrefixes() []tree.Datums { } type optTableStat struct { - createdAt time.Time + stat *stats.TableStatistic columnOrdinals []int - rowCount uint64 - distinctCount uint64 - nullCount uint64 - histogram []cat.HistogramBucket } var _ cat.TableStatistic = &optTableStat{} func (os *optTableStat) init(tab *optTable, stat *stats.TableStatistic) (ok bool, _ error) { - os.createdAt = stat.CreatedAt - os.rowCount = stat.RowCount - os.distinctCount = stat.DistinctCount - os.nullCount = stat.NullCount - + os.stat = stat os.columnOrdinals = make([]int, len(stat.ColumnIDs)) for i, c := range stat.ColumnIDs { var ok bool @@ -1028,31 +1019,13 @@ func (os *optTableStat) init(tab *optTable, stat *stats.TableStatistic) (ok bool } } - if stat.Histogram != nil { - os.histogram = make([]cat.HistogramBucket, len(stat.Histogram.Buckets)) - typ := &stat.Histogram.ColumnType - var a sqlbase.DatumAlloc - for i := range os.histogram { - bucket := &stat.Histogram.Buckets[i] - datum, _, err := sqlbase.DecodeTableKey(&a, typ, bucket.UpperBound, encoding.Ascending) - if err != nil { - return false, err - } - os.histogram[i] = cat.HistogramBucket{ - NumEq: float64(bucket.NumEq), - NumRange: float64(bucket.NumRange), - DistinctRange: bucket.DistinctRange, - UpperBound: datum, - } - } - } return true, nil } func (os *optTableStat) equals(other *optTableStat) bool { // Two table statistics are considered equal if they have been created at the // same time, on the same set of columns. - if os.createdAt != other.createdAt || len(os.columnOrdinals) != len(other.columnOrdinals) { + if os.CreatedAt() != other.CreatedAt() || len(os.columnOrdinals) != len(other.columnOrdinals) { return false } for i, c := range os.columnOrdinals { @@ -1065,7 +1038,7 @@ func (os *optTableStat) equals(other *optTableStat) bool { // CreatedAt is part of the cat.TableStatistic interface. func (os *optTableStat) CreatedAt() time.Time { - return os.createdAt + return os.stat.CreatedAt } // ColumnCount is part of the cat.TableStatistic interface. @@ -1080,22 +1053,22 @@ func (os *optTableStat) ColumnOrdinal(i int) int { // RowCount is part of the cat.TableStatistic interface. func (os *optTableStat) RowCount() uint64 { - return os.rowCount + return os.stat.RowCount } // DistinctCount is part of the cat.TableStatistic interface. func (os *optTableStat) DistinctCount() uint64 { - return os.distinctCount + return os.stat.DistinctCount } // NullCount is part of the cat.TableStatistic interface. func (os *optTableStat) NullCount() uint64 { - return os.nullCount + return os.stat.NullCount } // Histogram is part of the cat.TableStatistic interface. func (os *optTableStat) Histogram() []cat.HistogramBucket { - return os.histogram + return os.stat.Histogram } // optFamily is a wrapper around sqlbase.ColumnFamilyDescriptor that keeps a diff --git a/pkg/sql/stats/delete_stats_test.go b/pkg/sql/stats/delete_stats_test.go index d1c4f472ade2..1c3cd9990084 100644 --- a/pkg/sql/stats/delete_stats_test.go +++ b/pkg/sql/stats/delete_stats_test.go @@ -38,7 +38,7 @@ func TestDeleteOldStatsForColumns(t *testing.T) { // The test data must be ordered by CreatedAt DESC so the calculated set of // expected deleted stats is correct. - testData := []TableStatistic{ + testData := []TableStatisticProto{ { TableID: sqlbase.ID(100), StatisticID: 1, diff --git a/pkg/sql/stats/new_stat.go b/pkg/sql/stats/new_stat.go index 52df26f907b3..5759ce2c039e 100644 --- a/pkg/sql/stats/new_stat.go +++ b/pkg/sql/stats/new_stat.go @@ -28,7 +28,7 @@ func InsertNewStats( ctx context.Context, executor sqlutil.InternalExecutor, txn *client.Txn, - tableStats []*TableStatistic, + tableStats []*TableStatisticProto, ) error { var err error for _, statistic := range tableStats { @@ -42,7 +42,7 @@ func InsertNewStats( int64(statistic.RowCount), int64(statistic.DistinctCount), int64(statistic.NullCount), - statistic.Histogram, + statistic.HistogramData, ) if err != nil { return err diff --git a/pkg/sql/stats/stats_cache.go b/pkg/sql/stats/stats_cache.go index fa23631b3257..e5f1d0964762 100644 --- a/pkg/sql/stats/stats_cache.go +++ b/pkg/sql/stats/stats_cache.go @@ -17,17 +17,28 @@ import ( "github.com/cockroachdb/cockroach/pkg/gossip" "github.com/cockroachdb/cockroach/pkg/internal/client" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/sql/opt/cat" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" "github.com/cockroachdb/cockroach/pkg/sql/sqlutil" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/cache" + "github.com/cockroachdb/cockroach/pkg/util/encoding" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/protoutil" "github.com/cockroachdb/cockroach/pkg/util/syncutil" "github.com/pkg/errors" ) +// A TableStatistic object holds a statistic for a particular column or group +// of columns. +type TableStatistic struct { + TableStatisticProto + + // Histogram is the decoded histogram data. + Histogram []cat.HistogramBucket +} + // A TableStatisticsCache contains two underlying LRU caches: // (1) A cache of []*TableStatistic objects, keyed by table ID. // Each entry consists of all the statistics for different columns and @@ -262,33 +273,53 @@ func parseStats(datums tree.Datums) (*TableStatistic, error) { } // Extract datum values. - tableStatistic := &TableStatistic{ - TableID: sqlbase.ID((int32)(*datums[tableIDIndex].(*tree.DInt))), - StatisticID: (uint64)(*datums[statisticsIDIndex].(*tree.DInt)), - CreatedAt: datums[createdAtIndex].(*tree.DTimestamp).Time, - RowCount: (uint64)(*datums[rowCountIndex].(*tree.DInt)), - DistinctCount: (uint64)(*datums[distinctCountIndex].(*tree.DInt)), - NullCount: (uint64)(*datums[nullCountIndex].(*tree.DInt)), + res := &TableStatistic{ + TableStatisticProto: TableStatisticProto{ + TableID: sqlbase.ID((int32)(*datums[tableIDIndex].(*tree.DInt))), + StatisticID: (uint64)(*datums[statisticsIDIndex].(*tree.DInt)), + CreatedAt: datums[createdAtIndex].(*tree.DTimestamp).Time, + RowCount: (uint64)(*datums[rowCountIndex].(*tree.DInt)), + DistinctCount: (uint64)(*datums[distinctCountIndex].(*tree.DInt)), + NullCount: (uint64)(*datums[nullCountIndex].(*tree.DInt)), + }, } columnIDs := datums[columnIDsIndex].(*tree.DArray) - tableStatistic.ColumnIDs = make([]sqlbase.ColumnID, len(columnIDs.Array)) + res.ColumnIDs = make([]sqlbase.ColumnID, len(columnIDs.Array)) for i, d := range columnIDs.Array { - tableStatistic.ColumnIDs[i] = sqlbase.ColumnID((int32)(*d.(*tree.DInt))) + res.ColumnIDs[i] = sqlbase.ColumnID((int32)(*d.(*tree.DInt))) } if datums[nameIndex] != tree.DNull { - tableStatistic.Name = string(*datums[nameIndex].(*tree.DString)) + res.Name = string(*datums[nameIndex].(*tree.DString)) } if datums[histogramIndex] != tree.DNull { - tableStatistic.Histogram = &HistogramData{} + res.HistogramData = &HistogramData{} if err := protoutil.Unmarshal( []byte(*datums[histogramIndex].(*tree.DBytes)), - tableStatistic.Histogram, + res.HistogramData, ); err != nil { return nil, err } + + // Decode the histogram data so that it's usable by the opt catalog. + res.Histogram = make([]cat.HistogramBucket, len(res.HistogramData.Buckets)) + typ := &res.HistogramData.ColumnType + var a sqlbase.DatumAlloc + for i := range res.Histogram { + bucket := &res.HistogramData.Buckets[i] + datum, _, err := sqlbase.DecodeTableKey(&a, typ, bucket.UpperBound, encoding.Ascending) + if err != nil { + return nil, err + } + res.Histogram[i] = cat.HistogramBucket{ + NumEq: float64(bucket.NumEq), + NumRange: float64(bucket.NumRange), + DistinctRange: bucket.DistinctRange, + UpperBound: datum, + } + } } - return tableStatistic, nil + return res, nil } // getTableStatsFromDB retrieves the statistics in system.table_statistics diff --git a/pkg/sql/stats/stats_cache_test.go b/pkg/sql/stats/stats_cache_test.go index 0657f719791c..343fd87f91f7 100644 --- a/pkg/sql/stats/stats_cache_test.go +++ b/pkg/sql/stats/stats_cache_test.go @@ -34,7 +34,7 @@ import ( ) func insertTableStat( - ctx context.Context, db *client.DB, ex sqlutil.InternalExecutor, stat *TableStatistic, + ctx context.Context, db *client.DB, ex sqlutil.InternalExecutor, stat *TableStatisticProto, ) error { insertStatStmt := ` INSERT INTO system.table_statistics ("tableID", "statisticID", name, "columnIDs", "createdAt", @@ -62,8 +62,8 @@ VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) if len(stat.Name) != 0 { args[2] = stat.Name } - if stat.Histogram != nil { - histogramBytes, err := protoutil.Marshal(stat.Histogram) + if stat.HistogramData != nil { + histogramBytes, err := protoutil.Marshal(stat.HistogramData) if err != nil { return err } @@ -94,7 +94,10 @@ func lookupTableStats( } func checkStatsForTable( - ctx context.Context, sc *TableStatisticsCache, expected []*TableStatistic, tableID sqlbase.ID, + ctx context.Context, + sc *TableStatisticsCache, + expected []*TableStatisticProto, + tableID sqlbase.ID, ) error { // Initially the stats won't be in the cache. if statsList, ok := lookupTableStats(ctx, sc, tableID); ok { @@ -107,7 +110,7 @@ func checkStatsForTable( if err != nil { return errors.Errorf(err.Error()) } - if !reflect.DeepEqual(statsList, expected) { + if !checkStats(statsList, expected) { return errors.Errorf("for lookup of key %d, expected stats %s, got %s", tableID, expected, statsList) } @@ -118,12 +121,24 @@ func checkStatsForTable( return nil } +func checkStats(actual []*TableStatistic, expected []*TableStatisticProto) bool { + if len(actual) == 0 && len(expected) == 0 { + // DeepEqual differentiates between nil and empty slices, we don't. + return true + } + var protoList []*TableStatisticProto + for i := range actual { + protoList = append(protoList, &actual[i].TableStatisticProto) + } + return reflect.DeepEqual(protoList, expected) +} + func initTestData( ctx context.Context, db *client.DB, ex sqlutil.InternalExecutor, -) (map[sqlbase.ID][]*TableStatistic, error) { +) (map[sqlbase.ID][]*TableStatisticProto, error) { // The expected stats must be ordered by TableID+, CreatedAt- so they can // later be compared with the returned stats using reflect.DeepEqual. - expStatsList := []TableStatistic{ + expStatsList := []TableStatisticProto{ { TableID: sqlbase.ID(100), StatisticID: 0, @@ -133,7 +148,7 @@ func initTestData( RowCount: 32, DistinctCount: 30, NullCount: 0, - Histogram: &HistogramData{ColumnType: *types.Int, Buckets: []HistogramData_Bucket{ + HistogramData: &HistogramData{ColumnType: *types.Int, Buckets: []HistogramData_Bucket{ {NumEq: 3, NumRange: 30, UpperBound: encoding.EncodeVarintAscending(nil, 3000)}}, }, }, @@ -169,7 +184,7 @@ func initTestData( // Insert the stats into system.table_statistics // and store them in maps for fast retrieval. - expectedStats := make(map[sqlbase.ID][]*TableStatistic) + expectedStats := make(map[sqlbase.ID][]*TableStatisticProto) for i := range expStatsList { stat := &expStatsList[i] @@ -284,7 +299,7 @@ func TestCacheWait(t *testing.T) { stats, err := sc.GetTableStats(ctx, id) if err != nil { t.Error(err) - } else if !reflect.DeepEqual(stats, expectedStats[id]) { + } else if !checkStats(stats, expectedStats[id]) { t.Errorf("for table %d, expected stats %s, got %s", id, expectedStats[id], stats) } wg.Done() diff --git a/pkg/sql/stats/table_statistic.pb.go b/pkg/sql/stats/table_statistic.pb.go index a31b774c2afb..e845b98971c5 100644 --- a/pkg/sql/stats/table_statistic.pb.go +++ b/pkg/sql/stats/table_statistic.pb.go @@ -26,11 +26,11 @@ var _ = time.Kitchen // proto package needs to be updated. const _ = proto.GoGoProtoPackageIsVersion2 // please upgrade the proto package -// A TableStatistic object holds a statistic for a particular column or group -// of columns. It mirrors the structure of the system.table_statistics table. -// It is also used as the format in which table statistics are serialized -// in a backup. -type TableStatistic struct { +// A TableStatisticProto object holds a statistic for a particular column or +// group of columns. It mirrors the structure of the system.table_statistics +// table. It is also used as the format in which table statistics are +// serialized in a backup. +type TableStatisticProto struct { // The ID of the table. TableID github_com_cockroachdb_cockroach_pkg_sql_sqlbase.ID `protobuf:"varint,1,opt,name=table_id,json=tableId,proto3,casttype=github.com/cockroachdb/cockroach/pkg/sql/sqlbase.ID" json:"table_id,omitempty"` // The ID for this statistic. It need not be globally unique, @@ -49,19 +49,19 @@ type TableStatistic struct { // The number of rows that have a NULL in any of the columns in ColumnIDs. NullCount uint64 `protobuf:"varint,8,opt,name=null_count,json=nullCount,proto3" json:"null_count,omitempty"` // Histogram (if available) - Histogram *HistogramData `protobuf:"bytes,9,opt,name=histogram,proto3" json:"histogram,omitempty"` + HistogramData *HistogramData `protobuf:"bytes,9,opt,name=histogram_data,json=histogramData,proto3" json:"histogram_data,omitempty"` } -func (m *TableStatistic) Reset() { *m = TableStatistic{} } -func (m *TableStatistic) String() string { return proto.CompactTextString(m) } -func (*TableStatistic) ProtoMessage() {} -func (*TableStatistic) Descriptor() ([]byte, []int) { - return fileDescriptor_table_statistic_fb3b5153f9b658f1, []int{0} +func (m *TableStatisticProto) Reset() { *m = TableStatisticProto{} } +func (m *TableStatisticProto) String() string { return proto.CompactTextString(m) } +func (*TableStatisticProto) ProtoMessage() {} +func (*TableStatisticProto) Descriptor() ([]byte, []int) { + return fileDescriptor_table_statistic_a8d57cc4b836a89f, []int{0} } -func (m *TableStatistic) XXX_Unmarshal(b []byte) error { +func (m *TableStatisticProto) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) } -func (m *TableStatistic) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { +func (m *TableStatisticProto) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { b = b[:cap(b)] n, err := m.MarshalTo(b) if err != nil { @@ -69,22 +69,22 @@ func (m *TableStatistic) XXX_Marshal(b []byte, deterministic bool) ([]byte, erro } return b[:n], nil } -func (dst *TableStatistic) XXX_Merge(src proto.Message) { - xxx_messageInfo_TableStatistic.Merge(dst, src) +func (dst *TableStatisticProto) XXX_Merge(src proto.Message) { + xxx_messageInfo_TableStatisticProto.Merge(dst, src) } -func (m *TableStatistic) XXX_Size() int { +func (m *TableStatisticProto) XXX_Size() int { return m.Size() } -func (m *TableStatistic) XXX_DiscardUnknown() { - xxx_messageInfo_TableStatistic.DiscardUnknown(m) +func (m *TableStatisticProto) XXX_DiscardUnknown() { + xxx_messageInfo_TableStatisticProto.DiscardUnknown(m) } -var xxx_messageInfo_TableStatistic proto.InternalMessageInfo +var xxx_messageInfo_TableStatisticProto proto.InternalMessageInfo func init() { - proto.RegisterType((*TableStatistic)(nil), "cockroach.sql.stats.TableStatistic") + proto.RegisterType((*TableStatisticProto)(nil), "cockroach.sql.stats.TableStatisticProto") } -func (m *TableStatistic) Marshal() (dAtA []byte, err error) { +func (m *TableStatisticProto) Marshal() (dAtA []byte, err error) { size := m.Size() dAtA = make([]byte, size) n, err := m.MarshalTo(dAtA) @@ -94,7 +94,7 @@ func (m *TableStatistic) Marshal() (dAtA []byte, err error) { return dAtA[:n], nil } -func (m *TableStatistic) MarshalTo(dAtA []byte) (int, error) { +func (m *TableStatisticProto) MarshalTo(dAtA []byte) (int, error) { var i int _ = i var l int @@ -155,11 +155,11 @@ func (m *TableStatistic) MarshalTo(dAtA []byte) (int, error) { i++ i = encodeVarintTableStatistic(dAtA, i, uint64(m.NullCount)) } - if m.Histogram != nil { + if m.HistogramData != nil { dAtA[i] = 0x4a i++ - i = encodeVarintTableStatistic(dAtA, i, uint64(m.Histogram.Size())) - n4, err := m.Histogram.MarshalTo(dAtA[i:]) + i = encodeVarintTableStatistic(dAtA, i, uint64(m.HistogramData.Size())) + n4, err := m.HistogramData.MarshalTo(dAtA[i:]) if err != nil { return 0, err } @@ -177,7 +177,7 @@ func encodeVarintTableStatistic(dAtA []byte, offset int, v uint64) int { dAtA[offset] = uint8(v) return offset + 1 } -func (m *TableStatistic) Size() (n int) { +func (m *TableStatisticProto) Size() (n int) { if m == nil { return 0 } @@ -211,8 +211,8 @@ func (m *TableStatistic) Size() (n int) { if m.NullCount != 0 { n += 1 + sovTableStatistic(uint64(m.NullCount)) } - if m.Histogram != nil { - l = m.Histogram.Size() + if m.HistogramData != nil { + l = m.HistogramData.Size() n += 1 + l + sovTableStatistic(uint64(l)) } return n @@ -231,7 +231,7 @@ func sovTableStatistic(x uint64) (n int) { func sozTableStatistic(x uint64) (n int) { return sovTableStatistic(uint64((x << 1) ^ uint64((int64(x) >> 63)))) } -func (m *TableStatistic) Unmarshal(dAtA []byte) error { +func (m *TableStatisticProto) Unmarshal(dAtA []byte) error { l := len(dAtA) iNdEx := 0 for iNdEx < l { @@ -254,10 +254,10 @@ func (m *TableStatistic) Unmarshal(dAtA []byte) error { fieldNum := int32(wire >> 3) wireType := int(wire & 0x7) if wireType == 4 { - return fmt.Errorf("proto: TableStatistic: wiretype end group for non-group") + return fmt.Errorf("proto: TableStatisticProto: wiretype end group for non-group") } if fieldNum <= 0 { - return fmt.Errorf("proto: TableStatistic: illegal tag %d (wire type %d)", fieldNum, wire) + return fmt.Errorf("proto: TableStatisticProto: illegal tag %d (wire type %d)", fieldNum, wire) } switch fieldNum { case 1: @@ -489,7 +489,7 @@ func (m *TableStatistic) Unmarshal(dAtA []byte) error { } case 9: if wireType != 2 { - return fmt.Errorf("proto: wrong wireType = %d for field Histogram", wireType) + return fmt.Errorf("proto: wrong wireType = %d for field HistogramData", wireType) } var msglen int for shift := uint(0); ; shift += 7 { @@ -513,10 +513,10 @@ func (m *TableStatistic) Unmarshal(dAtA []byte) error { if postIndex > l { return io.ErrUnexpectedEOF } - if m.Histogram == nil { - m.Histogram = &HistogramData{} + if m.HistogramData == nil { + m.HistogramData = &HistogramData{} } - if err := m.Histogram.Unmarshal(dAtA[iNdEx:postIndex]); err != nil { + if err := m.HistogramData.Unmarshal(dAtA[iNdEx:postIndex]); err != nil { return err } iNdEx = postIndex @@ -647,38 +647,38 @@ var ( ) func init() { - proto.RegisterFile("sql/stats/table_statistic.proto", fileDescriptor_table_statistic_fb3b5153f9b658f1) + proto.RegisterFile("sql/stats/table_statistic.proto", fileDescriptor_table_statistic_a8d57cc4b836a89f) } -var fileDescriptor_table_statistic_fb3b5153f9b658f1 = []byte{ - // 452 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x94, 0x92, 0xbf, 0x8e, 0x9b, 0x40, - 0x10, 0xc6, 0xd9, 0x9c, 0xef, 0x6c, 0xd6, 0xf1, 0x45, 0xda, 0xa4, 0x20, 0x8e, 0xc2, 0xa2, 0x93, - 0xa2, 0x50, 0x2d, 0xd2, 0x5d, 0x95, 0x2e, 0xc1, 0x14, 0x21, 0x25, 0xb9, 0x2a, 0x52, 0x64, 0x2d, - 0x0b, 0xc1, 0xe8, 0x80, 0xb5, 0xd9, 0x45, 0xf7, 0x12, 0x29, 0xee, 0xb1, 0x5c, 0x5e, 0x79, 0x15, - 0x49, 0xf0, 0x5b, 0xb8, 0x8a, 0x58, 0xfe, 0xb8, 0x49, 0x93, 0x6e, 0x76, 0xe6, 0x37, 0x33, 0xdf, - 0x37, 0x00, 0xb1, 0xd8, 0x65, 0x8e, 0x90, 0x54, 0x0a, 0x47, 0xd2, 0x30, 0x8b, 0xd7, 0x6d, 0x9c, - 0x0a, 0x99, 0x32, 0xb2, 0x2d, 0xb9, 0xe4, 0xe8, 0x25, 0xe3, 0xec, 0xae, 0xe4, 0x94, 0x6d, 0x88, - 0xd8, 0x65, 0x44, 0xa1, 0xcb, 0x57, 0x09, 0x4f, 0xb8, 0xaa, 0x3b, 0x6d, 0xd4, 0xa1, 0x4b, 0x9c, - 0x70, 0x9e, 0x64, 0xb1, 0xa3, 0x5e, 0x61, 0xf5, 0xc3, 0x91, 0x69, 0x1e, 0x0b, 0x49, 0xf3, 0x6d, - 0x0f, 0xbc, 0x3e, 0x2d, 0xdb, 0xa4, 0x42, 0xf2, 0xa4, 0xa4, 0x79, 0x57, 0xba, 0xfa, 0x39, 0x81, - 0x97, 0xb7, 0xad, 0x80, 0xaf, 0xc3, 0x7e, 0xf4, 0x1d, 0xce, 0x3a, 0x49, 0x69, 0x64, 0x00, 0x0b, - 0xd8, 0x0b, 0xd7, 0x6d, 0x6a, 0x3c, 0x55, 0x94, 0xef, 0x1d, 0x6b, 0x7c, 0x93, 0xa4, 0x72, 0x53, - 0x85, 0x84, 0xf1, 0xdc, 0x19, 0x55, 0x46, 0xe1, 0x29, 0x76, 0xb6, 0x77, 0x89, 0xa3, 0x76, 0xee, - 0xb2, 0x90, 0x8a, 0x98, 0xf8, 0x5e, 0x30, 0x55, 0x33, 0xfd, 0x08, 0x5d, 0xc3, 0xe7, 0xa3, 0xd7, - 0x76, 0xc5, 0x33, 0x0b, 0xd8, 0x13, 0xf7, 0x45, 0x53, 0xe3, 0xf9, 0xa8, 0xc1, 0xf7, 0x82, 0xf9, - 0x08, 0xf9, 0x11, 0x42, 0x70, 0x52, 0xd0, 0x3c, 0x36, 0xce, 0x2c, 0x60, 0xeb, 0x81, 0x8a, 0x51, - 0x0a, 0x21, 0xe3, 0x59, 0x95, 0x17, 0xeb, 0x34, 0x12, 0xc6, 0xc4, 0x3a, 0xb3, 0x17, 0xee, 0x97, - 0xa6, 0xc6, 0xfa, 0x4a, 0x65, 0x7d, 0x4f, 0x1c, 0x6b, 0xfc, 0xe1, 0xbf, 0xa5, 0x0e, 0xdd, 0x81, - 0xde, 0x4d, 0xf7, 0x23, 0x81, 0x56, 0x10, 0xb2, 0x32, 0xa6, 0x32, 0x8e, 0xd6, 0x54, 0x1a, 0xe7, - 0x16, 0xb0, 0xe7, 0xd7, 0x4b, 0xd2, 0x5d, 0x9d, 0x0c, 0x57, 0x27, 0xb7, 0xc3, 0xd5, 0xdd, 0xd9, - 0xbe, 0xc6, 0xda, 0xc3, 0x2f, 0x0c, 0x02, 0xbd, 0xef, 0xfb, 0x24, 0xd1, 0x1b, 0xa8, 0x97, 0xfc, - 0x7e, 0xcd, 0x78, 0x55, 0x48, 0xe3, 0xa2, 0x35, 0x1d, 0xcc, 0x4a, 0x7e, 0xbf, 0x6a, 0xdf, 0xe8, - 0x1d, 0xbc, 0x8c, 0x5a, 0xb3, 0x05, 0x93, 0x3d, 0x31, 0x55, 0xc4, 0x62, 0xc8, 0x76, 0xd8, 0x5b, - 0x08, 0x8b, 0x2a, 0xcb, 0x7a, 0x64, 0xa6, 0x10, 0xbd, 0xcd, 0x74, 0xe5, 0x8f, 0x50, 0x1f, 0xbf, - 0xaf, 0xa1, 0x2b, 0x99, 0x57, 0xe4, 0x1f, 0xff, 0x11, 0xf9, 0x3c, 0x50, 0x1e, 0x95, 0x34, 0x38, - 0x35, 0xb9, 0xef, 0xf7, 0x7f, 0x4c, 0x6d, 0xdf, 0x98, 0xe0, 0xb1, 0x31, 0xc1, 0x53, 0x63, 0x82, - 0xdf, 0x8d, 0x09, 0x1e, 0x0e, 0xa6, 0xf6, 0x78, 0x30, 0xb5, 0xa7, 0x83, 0xa9, 0x7d, 0x3b, 0x57, - 0x13, 0xc2, 0x0b, 0x65, 0xfb, 0xe6, 0x6f, 0x00, 0x00, 0x00, 0xff, 0xff, 0xfd, 0xbe, 0x83, 0x6a, - 0xc8, 0x02, 0x00, 0x00, +var fileDescriptor_table_statistic_a8d57cc4b836a89f = []byte{ + // 459 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x94, 0x92, 0x3f, 0x6f, 0x9b, 0x40, + 0x18, 0xc6, 0xb9, 0xc6, 0x89, 0xcd, 0xb9, 0x4e, 0x25, 0xd2, 0x81, 0xba, 0x2a, 0x87, 0x22, 0x55, + 0x65, 0x3a, 0xa4, 0x64, 0xea, 0x58, 0xcc, 0x50, 0x3a, 0x55, 0x34, 0x53, 0xa5, 0x0a, 0x1d, 0x77, + 0x14, 0xa3, 0x00, 0x67, 0x73, 0x87, 0xf2, 0x35, 0x32, 0xf7, 0x13, 0x79, 0xcc, 0x98, 0x89, 0xb6, + 0xf8, 0x5b, 0x64, 0xaa, 0x38, 0x0c, 0xee, 0xd0, 0xa5, 0xdb, 0xfb, 0xe7, 0x77, 0xef, 0xf3, 0xbc, + 0x2f, 0x40, 0x24, 0xb6, 0xb9, 0x2b, 0x24, 0x91, 0xc2, 0x95, 0x24, 0xce, 0x93, 0xa8, 0x8b, 0x33, + 0x21, 0x33, 0x8a, 0x37, 0x15, 0x97, 0xdc, 0xb8, 0xa0, 0x9c, 0xde, 0x56, 0x9c, 0xd0, 0x35, 0x16, + 0xdb, 0x1c, 0x2b, 0x74, 0xf9, 0x32, 0xe5, 0x29, 0x57, 0x7d, 0xb7, 0x8b, 0x7a, 0x74, 0x89, 0x52, + 0xce, 0xd3, 0x3c, 0x71, 0x55, 0x16, 0xd7, 0xdf, 0x5d, 0x99, 0x15, 0x89, 0x90, 0xa4, 0xd8, 0x1c, + 0x80, 0x57, 0x47, 0xb1, 0x75, 0x26, 0x24, 0x4f, 0x2b, 0x52, 0xf4, 0xad, 0xcb, 0x1f, 0x13, 0x78, + 0x71, 0xd3, 0x19, 0xf8, 0x32, 0xe8, 0x7f, 0x56, 0xf2, 0xdf, 0xe0, 0xac, 0xf7, 0x95, 0x31, 0x13, + 0xd8, 0xc0, 0x59, 0x78, 0x5e, 0xdb, 0xa0, 0xa9, 0x42, 0x03, 0xff, 0xa9, 0x41, 0xd7, 0x69, 0x26, + 0xd7, 0x75, 0x8c, 0x29, 0x2f, 0xdc, 0xd1, 0x2a, 0x8b, 0x8f, 0xb1, 0xbb, 0xb9, 0x4d, 0x5d, 0x25, + 0xbc, 0xcd, 0x63, 0x22, 0x12, 0x1c, 0xf8, 0xe1, 0x54, 0xcd, 0x0c, 0x98, 0x71, 0x05, 0x9f, 0x8f, + 0x0b, 0x77, 0x12, 0xcf, 0x6c, 0xe0, 0x4c, 0xbc, 0x17, 0x6d, 0x83, 0xe6, 0xa3, 0x91, 0xc0, 0x0f, + 0xe7, 0x23, 0x14, 0x30, 0xc3, 0x80, 0x93, 0x92, 0x14, 0x89, 0x79, 0x62, 0x03, 0x47, 0x0f, 0x55, + 0x6c, 0x64, 0x10, 0x52, 0x9e, 0xd7, 0x45, 0x19, 0x65, 0x4c, 0x98, 0x13, 0xfb, 0xc4, 0x59, 0x78, + 0x9f, 0xda, 0x06, 0xe9, 0x2b, 0x55, 0x0d, 0x7c, 0xf1, 0xd4, 0xa0, 0xf7, 0xff, 0x6d, 0x75, 0x78, + 0x1d, 0xea, 0xfd, 0xf4, 0x80, 0x09, 0x63, 0x05, 0x21, 0xad, 0x12, 0x22, 0x13, 0x16, 0x11, 0x69, + 0x9e, 0xda, 0xc0, 0x99, 0x5f, 0x2d, 0x71, 0x7f, 0x7a, 0x3c, 0x9c, 0x1e, 0xdf, 0x0c, 0xa7, 0xf7, + 0x66, 0xbb, 0x06, 0x69, 0xf7, 0x3f, 0x11, 0x08, 0xf5, 0xc3, 0xbb, 0x0f, 0xd2, 0x78, 0x0d, 0xf5, + 0x8a, 0xdf, 0x45, 0x94, 0xd7, 0xa5, 0x34, 0xcf, 0xba, 0xa5, 0xc3, 0x59, 0xc5, 0xef, 0x56, 0x5d, + 0x6e, 0xbc, 0x85, 0xe7, 0xac, 0x5b, 0xb6, 0xa4, 0xf2, 0x40, 0x4c, 0x15, 0xb1, 0x18, 0xaa, 0x3d, + 0xf6, 0x06, 0xc2, 0xb2, 0xce, 0xf3, 0x03, 0x32, 0x53, 0x88, 0xde, 0x55, 0xfa, 0x76, 0x00, 0xcf, + 0xc7, 0x8f, 0x1c, 0x31, 0x22, 0x89, 0xa9, 0x2b, 0xaf, 0x97, 0xf8, 0x1f, 0x7f, 0x14, 0xfe, 0x38, + 0xa0, 0x3e, 0x91, 0x24, 0x5c, 0xac, 0xff, 0x4e, 0xbd, 0x77, 0xbb, 0xdf, 0x96, 0xb6, 0x6b, 0x2d, + 0xf0, 0xd0, 0x5a, 0xe0, 0xb1, 0xb5, 0xc0, 0xaf, 0xd6, 0x02, 0xf7, 0x7b, 0x4b, 0x7b, 0xd8, 0x5b, + 0xda, 0xe3, 0xde, 0xd2, 0xbe, 0x9e, 0xaa, 0x29, 0xf1, 0x99, 0xda, 0xff, 0xfa, 0x4f, 0x00, 0x00, + 0x00, 0xff, 0xff, 0xf1, 0xca, 0xa3, 0xde, 0xd6, 0x02, 0x00, 0x00, } diff --git a/pkg/sql/stats/table_statistic.proto b/pkg/sql/stats/table_statistic.proto index dea8d78d5939..e6e8b96db0c4 100644 --- a/pkg/sql/stats/table_statistic.proto +++ b/pkg/sql/stats/table_statistic.proto @@ -21,11 +21,11 @@ import "google/protobuf/timestamp.proto"; import "sql/stats/histogram.proto"; -// A TableStatistic object holds a statistic for a particular column or group -// of columns. It mirrors the structure of the system.table_statistics table. -// It is also used as the format in which table statistics are serialized -// in a backup. -message TableStatistic { +// A TableStatisticProto object holds a statistic for a particular column or +// group of columns. It mirrors the structure of the system.table_statistics +// table. It is also used as the format in which table statistics are +// serialized in a backup. +message TableStatisticProto { // The ID of the table. uint32 table_id = 1 [(gogoproto.customname) = "TableID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sqlbase.ID"]; @@ -46,5 +46,5 @@ message TableStatistic { // The number of rows that have a NULL in any of the columns in ColumnIDs. uint64 null_count = 8; // Histogram (if available) - HistogramData histogram = 9; + HistogramData histogram_data = 9; } From ab74a33a13b3c5b116b6b71a9e84e4ffdfd9183d Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Tue, 1 Oct 2019 17:53:17 +0200 Subject: [PATCH 2/2] util/log: ensure that secondary loggers do not leak memory Prior to this patch, logging via a secondary logger would allocate a buffer, then add it to the buffer free list of the secondary logger. This was causing a memory leak because only the free list from the main logger is used to allocate buffers (even in secondary loggers), so all the now-unused buffers from secondary logs would remain unused and accumulate, locked from Go's GC attention because they are referenced somewhere. Release justification: bug fix Release note (bug fix): A memory leak was fixed that affected secondary logging (SQL audit logs, statement execution, and RocksDB logging). --- pkg/util/log/clog.go | 45 ++++++++++++++++++++------------------- pkg/util/log/clog_test.go | 4 ++-- pkg/util/log/log.go | 2 +- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/pkg/util/log/clog.go b/pkg/util/log/clog.go index 7076c4c9e1b6..d1b4515c9d23 100644 --- a/pkg/util/log/clog.go +++ b/pkg/util/log/clog.go @@ -480,7 +480,7 @@ func formatHeader( cp = nil } - buf := logging.getBuffer() + buf := getBuffer() if line < 0 { line = 0 // not a real line number, but acceptable to someDigits } @@ -696,13 +696,6 @@ type loggingT struct { // Level flag for output to files. fileThreshold Severity - // freeList is a list of byte buffers, maintained under freeListMu. - freeList *buffer - // freeListMu maintains the free list. It is separate from the main mutex - // so buffers can be grabbed and printed to without holding the main lock, - // for better parallelization. - freeListMu syncutil.Mutex - // mu protects the remaining elements of this structure and is // used to synchronize logging. mu syncutil.Mutex @@ -742,6 +735,12 @@ type loggingT struct { clusterID string } +var freeList struct { + syncutil.Mutex + // head is the head of a list of byte buffers, maintained under the mutex. + head *buffer +} + // buffer holds a byte Buffer for reuse. The zero value is ready for use. type buffer struct { bytes.Buffer @@ -792,13 +791,13 @@ func (l *loggingT) setVState(verbosity level, filter []modulePat, setFilter bool } // getBuffer returns a new, ready-to-use buffer. -func (l *loggingT) getBuffer() *buffer { - l.freeListMu.Lock() - b := l.freeList +func getBuffer() *buffer { + freeList.Lock() + b := freeList.head if b != nil { - l.freeList = b.next + freeList.head = b.next } - l.freeListMu.Unlock() + freeList.Unlock() if b == nil { b = new(buffer) } else { @@ -809,15 +808,15 @@ func (l *loggingT) getBuffer() *buffer { } // putBuffer returns a buffer to the free list. -func (l *loggingT) putBuffer(b *buffer) { +func putBuffer(b *buffer) { if b.Len() >= 256 { // Let big buffers die a natural death. return } - l.freeListMu.Lock() - b.next = l.freeList - l.freeList = b - l.freeListMu.Unlock() + freeList.Lock() + b.next = freeList.head + freeList.head = b + freeList.Unlock() } // ensureFile ensures that l.file is set and valid. @@ -938,10 +937,11 @@ func (l *loggingT) outputLogEntry(s Severity, file string, line int, msg string) if err := l.writeToFile(data); err != nil { l.exitLocked(err) l.mu.Unlock() + putBuffer(buf) return } - l.putBuffer(buf) + putBuffer(buf) } // Flush and exit on fatal logging. if s == Severity_FATAL { @@ -987,10 +987,11 @@ func (l *loggingT) printPanicToFile(r interface{}) { func (l *loggingT) outputToStderr(entry Entry, stacks []byte) { buf := l.processForStderr(entry, stacks) - if _, err := OrigStderr.Write(buf.Bytes()); err != nil { + _, err := OrigStderr.Write(buf.Bytes()) + putBuffer(buf) + if err != nil { l.exitLocked(err) } - l.putBuffer(buf) } // processForStderr formats a log entry for output to standard error. @@ -1153,11 +1154,11 @@ func (sb *syncBuffer) rotateFile(now time.Time) error { }, nil, nil) var n int n, err = sb.file.Write(buf.Bytes()) + putBuffer(buf) sb.nbytes += int64(n) if err != nil { return err } - logging.putBuffer(buf) } select { diff --git a/pkg/util/log/clog_test.go b/pkg/util/log/clog_test.go index f447f3fb4b11..57c1d4b41e8c 100644 --- a/pkg/util/log/clog_test.go +++ b/pkg/util/log/clog_test.go @@ -142,7 +142,7 @@ func TestEntryDecoder(t *testing.T) { buf := formatHeader(s, now, gid, file, line, nil) buf.WriteString(msg) buf.WriteString("\n") - defer logging.putBuffer(buf) + defer putBuffer(buf) return buf.String() } @@ -720,7 +720,7 @@ func TestExitOnFullDisk(t *testing.T) { func BenchmarkHeader(b *testing.B) { for i := 0; i < b.N; i++ { buf := formatHeader(Severity_INFO, timeutil.Now(), 200, "file.go", 100, nil) - logging.putBuffer(buf) + putBuffer(buf) } } diff --git a/pkg/util/log/log.go b/pkg/util/log/log.go index 0338735107b3..ce79dc6c886f 100644 --- a/pkg/util/log/log.go +++ b/pkg/util/log/log.go @@ -257,7 +257,7 @@ func MakeEntry(s Severity, t int64, file string, line int, msg string) Entry { // Format writes the log entry to the specified writer. func (e Entry) Format(w io.Writer) error { buf := formatLogEntry(e, nil, nil) - defer logging.putBuffer(buf) + defer putBuffer(buf) _, err := w.Write(buf.Bytes()) return err }