proto: disable XXX_NoUnkeyedLiteral and XXX_sizecache fields #38404
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
ajwerner
approved these changes
Jun 25, 2019
There was a problem hiding this comment.
Reviewed 27 of 68 files at r1, 68 of 68 files at r2.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @RaduBerinde)
RaduBerinde
approved these changes
Jun 25, 2019
nvanbenschoten
force-pushed
the
nvanbenschoten/sizeCache
branch
3 times, most recently
from
265bcc9 to
9e30f1c
Compare
The field claims to come with a small perf win for proto marshalling, but it looks like it's only used with `proto.Marshal` and not with the `Marshal` method (which CRDB code and gRPC code always use). Meanwhile, the cache has two moderate disadvantages: 1. it adds 32 bits to every proto struct. This isn't a big deal in most cases, but in code that allocates large slices of little proto structs (like the `spanlatch.Manager`, which is what tipped me off to this new field), it has a non-negligible cost. 2. it makes value comparison between proto structs dangerously unreliable. We compare small protos by value all over the place (see `hlc.Timestamp`) and switching to an `Equals` method would come with a cost to code complexity and runtime performance. All in all, the `XXX_sizecache` field doesn't seem to be worth it. Release note: None
Because the field is now at the end of generated protos, it is now taking up space even though it is an empty struct: golang/go#9401 Since this is just a glorified lint which is already enforced by `go vet`'s “composites” check, it doesn't seem worth the cost. Release note: None
nvanbenschoten
force-pushed
the
nvanbenschoten/sizeCache
branch
from
9e30f1c to
95878e0
Compare
|
bors r+ |
Build failed |
With no trace in the logs. Assuming that's a flake because CI passed. bors r+ |
craig bot
pushed a commit
that referenced
this pull request
Jun 25, 2019
38404: proto: disable XXX_NoUnkeyedLiteral and XXX_sizecache fields r=nvanbenschoten a=nvanbenschoten Fixes #37706. The PR removes the `XXX_NoUnkeyedLiteral` and `XXX_sizecache` fields from generated protobufs. `XXX_NoUnkeyedLiteral` claims to come with a small perf win for proto marshaling, but it looks like it's only used with `proto.Marshal` and not with the `Marshal` method (which CRDB code and gRPC code always use). Meanwhile, it has two moderate disadvantages: 1. it adds 32 bits to every proto struct. This isn't a big deal in most cases, but in code that allocates large slices of little proto structs (like the `spanlatch.Manager`, which is what tipped me off to this new field), it has a non-negligible cost. 2. it makes value comparison between proto structs dangerously unreliable. We compare small protos by value all over the place (see `hlc.Timestamp`) and switching to an `Equals` method would come with a cost to code complexity and runtime performance. After removing `XXX_NoUnkeyedLiteral`, `XXX_sizecache` is now at the end of generated protos, it is now taking up space even though it is an empty struct: golang/go#9401. Since this is just a glorified lint which is already enforced by `go vet`'s “composites” check, it doesn't seem worth the cost. ### Benchmarks ``` name old reads/s new reads/s delta kv95/enc=false/nodes=3 35.5k ± 1% 35.8k ± 1% +0.86% (p=0.214 n=5+5) name old writes/s new writes/s delta kv95/enc=false/nodes=3 1.87k ± 2% 1.89k ± 2% +0.94% (p=0.214 n=5+5) ``` Co-authored-by: Nathan VanBenschoten <[email protected]>
Build succeeded |
nvanbenschoten
added a commit
to nvanbenschoten/etcd
that referenced
this pull request
Mar 20, 2021
…he fields in protos This commit removes the `XXX_NoUnkeyedLiteral`, `XXX_unrecognized`, and `XXX_sizecache` auto-generated fields from generated protobuf structs in the raft package. This was done for all of the same reasons CockroachDB removed the generation of these fields in cockroachdb/cockroach#38404. They come with very limited advantages but moderate disadvantages. `XXX_NoUnkeyedLiteral` and `XXX_sizecache` were only enabled recently in cc7b4fa, and this appears to have been unintentional. Meanwhile, `XXX_unrecognized` has been around for longer and has arguably more reason to stay because it can assist with forwards compatibility. However, any real mixed-version upgrade story for this package is mostly untold at this point, and keeping this field seems just as likely to cause unexpected bugs (e.g. a field was propagated but not updated correctly when passed through an old version) as it seems to fix real issues, so it also doesn't warrant its cost. This reduces the in-memory representation size of all Raft protos. Notably, it reduces the memory size of an `Entry` proto from *80 bytes* to *48 bytes* and the memory size of a `Message` proto from *392 bytes* to *264 bytes*. Both of these structs are used frequently, and often in slices, where this wasted space really starts to add up. This was motivated by a regression in microbenchmarks in CockroachDB due to cc7b4fa, which was caught in cockroachdb/cockroach#62212.
nvanbenschoten
added a commit
to nvanbenschoten/etcd
that referenced
this pull request
Mar 20, 2021
…he fields in protos This commit removes the `XXX_NoUnkeyedLiteral`, `XXX_unrecognized`, and `XXX_sizecache` auto-generated fields from generated protobuf structs in the raft package. This was done for all of the same reasons CockroachDB removed the generation of these fields in cockroachdb/cockroach#38404. They come with very limited advantages but moderate disadvantages. `XXX_NoUnkeyedLiteral` and `XXX_sizecache` were only enabled recently in cc7b4fa, and this appears to have been unintentional. Meanwhile, `XXX_unrecognized` has been around for longer and has arguably more reason to stay because it can assist with forwards compatibility. However, any real mixed-version upgrade story for this package is mostly untold at this point, and keeping this field seems just as likely to cause unexpected bugs (e.g. a field was propagated but not updated correctly when passed through an old version) as it seems to fix real issues, so it also doesn't warrant its cost. This reduces the in-memory representation size of all Raft protos. Notably, it reduces the memory size of an `Entry` proto from *80 bytes* to *48 bytes* and the memory size of a `Message` proto from *392 bytes* to *264 bytes*. Both of these structs are used frequently, and often in slices, where this wasted space really starts to add up. This was motivated by a regression in microbenchmarks in CockroachDB due to cc7b4fa, which was caught in cockroachdb/cockroach#62212.
ijsong
added a commit
to kakao/varlog
that referenced
this pull request
Aug 5, 2022
* *: add topic Added topic to Varlog. * proto: add Topic into proto (#479) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go * Update internal/metadata_repository/storage.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * fix code * fix code Co-authored-by: pharrell.jang <[email protected]> Co-authored-by: jun.song(송인준)/kakao <[email protected]> * *: use int32 for storage node id and log stream id (#481) Changed type of `types.StorageNodeID` and `types.LogStreamID` from uint32 to int32. Resolves [#VARLOG-548](https://jira.daumkakao.com/browse/VARLOG-548). * topic: managemant topic (#485) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * wip * management topic * add test for register topic * add test for unregister topic * fmt * fix code * fix test * fix code * fix code Co-authored-by: pharrell.jang <[email protected]> * sn: remove redundant types for replica (#483) There were redundant types to represent replica: - `pkg/logc/StorageNode` - `proto/snpb/Replica` - `proto/snpb/AppendRequest_BackupNode` This patch removes those and uses `proto/varlogpb/StorageNode` and types that wrap it. Resolves [#VARLOG-546](https://jira.daumkakao.com/browse/VARLOG-546). * sn: add topic id to log i/o (#486) This patch adds `TopicID` to the methods of Log I/O interface. It doesn't contain any meaningful implementations about `TopicID`. Types that now have `TopicID` are follows: - `internal/storagenode/logio.ReadWriter` - `internal/storagenode/reportcommitter/reportcommitter.Getter` - `proto/snpb.AppendRequest`, `proto/snpb.AppendResponse`, `proto/snpb.ReadRequest`, `proto/snpb.SubscribeRequest`, `proto/snpb.TrimRequest` - `proto/varlogpb.Replica` Resolves [#VARLOG-542](https://jira.daumkakao.com/browse/VARLOG-542). * it: fix flaky test - TestVarlogSubscribeWithAddLS (#487) The `TestVarlogSubscribeWithAddLS` created a goroutine adding new LogStream while appending log entries. It however did not manage the life cycle of the goroutine resulting in several issues: - use closed connection to append logs - wait for commit messages from MR indefinitely since the MR is already closed This patch simply adds `sync.WaitGroup` to the test to avoid the above issues. Resolves [#VARLOG-569](https://jira.daumkakao.com/browse/VARLOG-569). * proto: removing unnecessary fields from messages (#488) This patch removes unnecessary fields generated automatically from proto messages. These lines are like follows: ``` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` ``` They are something related to either optimizations or compatibility that are generated by the gogoproto. However, they come to small overhead in many cases. (See cockroachdb/cockroach#38404 and etcd-io/etcd@e51c697#diff-76a35072df72591a656e69cab6f6fa99aa386fd5ace35c9042851eb324ec16b5). This change adds the following options to every proto definition file: ``` option (gogoproto.goproto_unkeyed_all) = false; option (gogoproto.goproto_unrecognized_all) = false; option (gogoproto.goproto_sizecache_all) = false; ``` Resolves [#VARLOG-557](https://jira.daumkakao.com/browse/VARLOG-557). * *: dedup LogEntry types (#489) - Remove `pkg/types.LogEntry`, then use `proto/varlogpb.LogEntry`. Resolves [#VARLOG-558](https://jira.daumkakao.com/browse/VARLOG-558). * vendor: bump Pebble (#490) Resolves #VARLOG-556 * sn: rename AddLogStream RPC (#491) In this patch, the RPC `AddLogStream` renames to `AddLogStreamReplica` to clarify its behavior. The RPC `AddLogStreamReplica` adds a new replica of the given LogStream to the StorageNode. Resolves [#VARLOG-568](https://jira.daumkakao.com/browse/VARLOG-568). * topic: apply Topic into client (#493) * logio: apply topic * add topic test * fix TestMRTopicLastHighWatermark * fix managing log stream status in vms Resolves #VARLOG-559 * all: update golang 1.17.0 (#492) Resolves [#VARLOG-555](https://jira.daumkakao.com/browse/VARLOG-555). * all: fix code style (#494) This patch just fixes code styles. Resolves [#VARLOG-572](https://jira.daumkakao.com/browse/VARLOG-572). * build: use predefined protoc (#496) Resolves [#VARLOG-563](https://jira.daumkakao.com/browse/VARLOG-563). * sn,topic: checking topic id while handling RPCs (#495) This patch adds a feature that the StorageNode checks if the topic id is valid or not. To support this functionality it adds a topicID to a parameter of many functions. The executor does not care about the topicID, rather it will be considered by StorageNode. To do that, StorageNode maintains the executors by using the executorsmap keyed by logStreamTopicID. The logStreamTopicID is a packed type of the LogStreamID and TopicID. Resolves [#VARLOG-542](https://jira.daumkakao.com/browse/VARLOG-542). * lint: fix code style (#497) This is a follow-up PR for VARLOG-572. Resolves [#VARLOG-572](https://jira.daumkakao.com/browse/VARLOG-572). Co-authored-by: pharrell.jang(장형근)/kakao <[email protected]> Co-authored-by: pharrell.jang <[email protected]>
ijsong
added a commit
to kakao/varlog
that referenced
this pull request
Aug 8, 2022
* *: add topic Added topic to Varlog. * proto: add Topic into proto (#479) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go * Update internal/metadata_repository/storage.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * fix code * fix code Co-authored-by: pharrell.jang <[email protected]> Co-authored-by: jun.song(송인준)/kakao <[email protected]> * *: use int32 for storage node id and log stream id (#481) Changed type of `types.StorageNodeID` and `types.LogStreamID` from uint32 to int32. Resolves [#VARLOG-548](https://jira.daumkakao.com/browse/VARLOG-548). * topic: managemant topic (#485) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * wip * management topic * add test for register topic * add test for unregister topic * fmt * fix code * fix test * fix code * fix code Co-authored-by: pharrell.jang <[email protected]> * sn: remove redundant types for replica (#483) There were redundant types to represent replica: - `pkg/logc/StorageNode` - `proto/snpb/Replica` - `proto/snpb/AppendRequest_BackupNode` This patch removes those and uses `proto/varlogpb/StorageNode` and types that wrap it. Resolves [#VARLOG-546](https://jira.daumkakao.com/browse/VARLOG-546). * sn: add topic id to log i/o (#486) This patch adds `TopicID` to the methods of Log I/O interface. It doesn't contain any meaningful implementations about `TopicID`. Types that now have `TopicID` are follows: - `internal/storagenode/logio.ReadWriter` - `internal/storagenode/reportcommitter/reportcommitter.Getter` - `proto/snpb.AppendRequest`, `proto/snpb.AppendResponse`, `proto/snpb.ReadRequest`, `proto/snpb.SubscribeRequest`, `proto/snpb.TrimRequest` - `proto/varlogpb.Replica` Resolves [#VARLOG-542](https://jira.daumkakao.com/browse/VARLOG-542). * it: fix flaky test - TestVarlogSubscribeWithAddLS (#487) The `TestVarlogSubscribeWithAddLS` created a goroutine adding new LogStream while appending log entries. It however did not manage the life cycle of the goroutine resulting in several issues: - use closed connection to append logs - wait for commit messages from MR indefinitely since the MR is already closed This patch simply adds `sync.WaitGroup` to the test to avoid the above issues. Resolves [#VARLOG-569](https://jira.daumkakao.com/browse/VARLOG-569). * proto: removing unnecessary fields from messages (#488) This patch removes unnecessary fields generated automatically from proto messages. These lines are like follows: ``` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` ``` They are something related to either optimizations or compatibility that are generated by the gogoproto. However, they come to small overhead in many cases. (See cockroachdb/cockroach#38404 and etcd-io/etcd@e51c697#diff-76a35072df72591a656e69cab6f6fa99aa386fd5ace35c9042851eb324ec16b5). This change adds the following options to every proto definition file: ``` option (gogoproto.goproto_unkeyed_all) = false; option (gogoproto.goproto_unrecognized_all) = false; option (gogoproto.goproto_sizecache_all) = false; ``` Resolves [#VARLOG-557](https://jira.daumkakao.com/browse/VARLOG-557). * *: dedup LogEntry types (#489) - Remove `pkg/types.LogEntry`, then use `proto/varlogpb.LogEntry`. Resolves [#VARLOG-558](https://jira.daumkakao.com/browse/VARLOG-558). * vendor: bump Pebble (#490) Resolves #VARLOG-556 * sn: rename AddLogStream RPC (#491) In this patch, the RPC `AddLogStream` renames to `AddLogStreamReplica` to clarify its behavior. The RPC `AddLogStreamReplica` adds a new replica of the given LogStream to the StorageNode. Resolves [#VARLOG-568](https://jira.daumkakao.com/browse/VARLOG-568). * topic: apply Topic into client (#493) * logio: apply topic * add topic test * fix TestMRTopicLastHighWatermark * fix managing log stream status in vms Resolves #VARLOG-559 * all: update golang 1.17.0 (#492) Resolves [#VARLOG-555](https://jira.daumkakao.com/browse/VARLOG-555). * all: fix code style (#494) This patch just fixes code styles. Resolves [#VARLOG-572](https://jira.daumkakao.com/browse/VARLOG-572). * build: use predefined protoc (#496) Resolves [#VARLOG-563](https://jira.daumkakao.com/browse/VARLOG-563). * sn,topic: checking topic id while handling RPCs (#495) This patch adds a feature that the StorageNode checks if the topic id is valid or not. To support this functionality it adds a topicID to a parameter of many functions. The executor does not care about the topicID, rather it will be considered by StorageNode. To do that, StorageNode maintains the executors by using the executorsmap keyed by logStreamTopicID. The logStreamTopicID is a packed type of the LogStreamID and TopicID. Resolves [#VARLOG-542](https://jira.daumkakao.com/browse/VARLOG-542). * lint: fix code style (#497) This is a follow-up PR for VARLOG-572. Resolves [#VARLOG-572](https://jira.daumkakao.com/browse/VARLOG-572). Co-authored-by: pharrell.jang(장형근)/kakao <[email protected]> Co-authored-by: pharrell.jang <[email protected]>
ijsong
added a commit
to kakao/varlog
that referenced
this pull request
Aug 9, 2022
* *: add topic Added topic to Varlog. * proto: add Topic into proto (#479) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go * Update internal/metadata_repository/storage.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * fix code * fix code Co-authored-by: pharrell.jang <[email protected]> Co-authored-by: jun.song(송인준)/kakao <[email protected]> * *: use int32 for storage node id and log stream id (#481) Changed type of `types.StorageNodeID` and `types.LogStreamID` from uint32 to int32. Resolves [#VARLOG-548](VARLOG-548). * topic: managemant topic (#485) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * wip * management topic * add test for register topic * add test for unregister topic * fmt * fix code * fix test * fix code * fix code Co-authored-by: pharrell.jang <[email protected]> * sn: remove redundant types for replica (#483) There were redundant types to represent replica: - `pkg/logc/StorageNode` - `proto/snpb/Replica` - `proto/snpb/AppendRequest_BackupNode` This patch removes those and uses `proto/varlogpb/StorageNode` and types that wrap it. Resolves [#VARLOG-546](VARLOG-546). * sn: add topic id to log i/o (#486) This patch adds `TopicID` to the methods of Log I/O interface. It doesn't contain any meaningful implementations about `TopicID`. Types that now have `TopicID` are follows: - `internal/storagenode/logio.ReadWriter` - `internal/storagenode/reportcommitter/reportcommitter.Getter` - `proto/snpb.AppendRequest`, `proto/snpb.AppendResponse`, `proto/snpb.ReadRequest`, `proto/snpb.SubscribeRequest`, `proto/snpb.TrimRequest` - `proto/varlogpb.Replica` Resolves [#VARLOG-542](VARLOG-542). * it: fix flaky test - TestVarlogSubscribeWithAddLS (#487) The `TestVarlogSubscribeWithAddLS` created a goroutine adding new LogStream while appending log entries. It however did not manage the life cycle of the goroutine resulting in several issues: - use closed connection to append logs - wait for commit messages from MR indefinitely since the MR is already closed This patch simply adds `sync.WaitGroup` to the test to avoid the above issues. Resolves [#VARLOG-569](VARLOG-569). * proto: removing unnecessary fields from messages (#488) This patch removes unnecessary fields generated automatically from proto messages. These lines are like follows: ``` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` ``` They are something related to either optimizations or compatibility that are generated by the gogoproto. However, they come to small overhead in many cases. (See cockroachdb/cockroach#38404 and etcd-io/etcd@e51c697#diff-76a35072df72591a656e69cab6f6fa99aa386fd5ace35c9042851eb324ec16b5). This change adds the following options to every proto definition file: ``` option (gogoproto.goproto_unkeyed_all) = false; option (gogoproto.goproto_unrecognized_all) = false; option (gogoproto.goproto_sizecache_all) = false; ``` Resolves [#VARLOG-557](VARLOG-557). * *: dedup LogEntry types (#489) - Remove `pkg/types.LogEntry`, then use `proto/varlogpb.LogEntry`. Resolves [#VARLOG-558](VARLOG-558). * vendor: bump Pebble (#490) Resolves #VARLOG-556 * sn: rename AddLogStream RPC (#491) In this patch, the RPC `AddLogStream` renames to `AddLogStreamReplica` to clarify its behavior. The RPC `AddLogStreamReplica` adds a new replica of the given LogStream to the StorageNode. Resolves [#VARLOG-568](VARLOG-568). * topic: apply Topic into client (#493) * logio: apply topic * add topic test * fix TestMRTopicLastHighWatermark * fix managing log stream status in vms Resolves #VARLOG-559 * all: update golang 1.17.0 (#492) Resolves [#VARLOG-555](VARLOG-555). * all: fix code style (#494) This patch just fixes code styles. Resolves [#VARLOG-572](VARLOG-572). * build: use predefined protoc (#496) Resolves [#VARLOG-563](VARLOG-563). * sn,topic: checking topic id while handling RPCs (#495) This patch adds a feature that the StorageNode checks if the topic id is valid or not. To support this functionality it adds a topicID to a parameter of many functions. The executor does not care about the topicID, rather it will be considered by StorageNode. To do that, StorageNode maintains the executors by using the executorsmap keyed by logStreamTopicID. The logStreamTopicID is a packed type of the LogStreamID and TopicID. Resolves [#VARLOG-542](VARLOG-542). * lint: fix code style (#497) This is a follow-up PR for VARLOG-572. Resolves [#VARLOG-572](VARLOG-572). Co-authored-by: pharrell.jang(장형근)/kakao <[email protected]> Co-authored-by: pharrell.jang <[email protected]>
ijsong
added a commit
to kakao/varlog
that referenced
this pull request
Aug 9, 2022
* *: add topic Added topic to Varlog. * proto: add Topic into proto (#479) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go * Update internal/metadata_repository/storage.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * fix code * fix code Co-authored-by: pharrell.jang <[email protected]> Co-authored-by: jun.song(송인준)/kakao <[email protected]> * *: use int32 for storage node id and log stream id (#481) Changed type of `types.StorageNodeID` and `types.LogStreamID` from uint32 to int32. Resolves [#VARLOG-548](VARLOG-548). * topic: managemant topic (#485) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * wip * management topic * add test for register topic * add test for unregister topic * fmt * fix code * fix test * fix code * fix code Co-authored-by: pharrell.jang <[email protected]> * sn: remove redundant types for replica (#483) There were redundant types to represent replica: - `pkg/logc/StorageNode` - `proto/snpb/Replica` - `proto/snpb/AppendRequest_BackupNode` This patch removes those and uses `proto/varlogpb/StorageNode` and types that wrap it. Resolves [#VARLOG-546](VARLOG-546). * sn: add topic id to log i/o (#486) This patch adds `TopicID` to the methods of Log I/O interface. It doesn't contain any meaningful implementations about `TopicID`. Types that now have `TopicID` are follows: - `internal/storagenode/logio.ReadWriter` - `internal/storagenode/reportcommitter/reportcommitter.Getter` - `proto/snpb.AppendRequest`, `proto/snpb.AppendResponse`, `proto/snpb.ReadRequest`, `proto/snpb.SubscribeRequest`, `proto/snpb.TrimRequest` - `proto/varlogpb.Replica` Resolves [#VARLOG-542](VARLOG-542). * it: fix flaky test - TestVarlogSubscribeWithAddLS (#487) The `TestVarlogSubscribeWithAddLS` created a goroutine adding new LogStream while appending log entries. It however did not manage the life cycle of the goroutine resulting in several issues: - use closed connection to append logs - wait for commit messages from MR indefinitely since the MR is already closed This patch simply adds `sync.WaitGroup` to the test to avoid the above issues. Resolves [#VARLOG-569](VARLOG-569). * proto: removing unnecessary fields from messages (#488) This patch removes unnecessary fields generated automatically from proto messages. These lines are like follows: ``` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` ``` They are something related to either optimizations or compatibility that are generated by the gogoproto. However, they come to small overhead in many cases. (See cockroachdb/cockroach#38404 and etcd-io/etcd@e51c697#diff-76a35072df72591a656e69cab6f6fa99aa386fd5ace35c9042851eb324ec16b5). This change adds the following options to every proto definition file: ``` option (gogoproto.goproto_unkeyed_all) = false; option (gogoproto.goproto_unrecognized_all) = false; option (gogoproto.goproto_sizecache_all) = false; ``` Resolves [#VARLOG-557](VARLOG-557). * *: dedup LogEntry types (#489) - Remove `pkg/types.LogEntry`, then use `proto/varlogpb.LogEntry`. Resolves [#VARLOG-558](VARLOG-558). * vendor: bump Pebble (#490) Resolves #VARLOG-556 * sn: rename AddLogStream RPC (#491) In this patch, the RPC `AddLogStream` renames to `AddLogStreamReplica` to clarify its behavior. The RPC `AddLogStreamReplica` adds a new replica of the given LogStream to the StorageNode. Resolves [#VARLOG-568](VARLOG-568). * topic: apply Topic into client (#493) * logio: apply topic * add topic test * fix TestMRTopicLastHighWatermark * fix managing log stream status in vms Resolves #VARLOG-559 * all: update golang 1.17.0 (#492) Resolves [#VARLOG-555](VARLOG-555). * all: fix code style (#494) This patch just fixes code styles. Resolves [#VARLOG-572](VARLOG-572). * build: use predefined protoc (#496) Resolves [#VARLOG-563](VARLOG-563). * sn,topic: checking topic id while handling RPCs (#495) This patch adds a feature that the StorageNode checks if the topic id is valid or not. To support this functionality it adds a topicID to a parameter of many functions. The executor does not care about the topicID, rather it will be considered by StorageNode. To do that, StorageNode maintains the executors by using the executorsmap keyed by logStreamTopicID. The logStreamTopicID is a packed type of the LogStreamID and TopicID. Resolves [#VARLOG-542](VARLOG-542). * lint: fix code style (#497) This is a follow-up PR for VARLOG-572. Resolves [#VARLOG-572](VARLOG-572). Co-authored-by: pharrell.jang(장형근)/kakao <[email protected]> Co-authored-by: pharrell.jang <[email protected]>
ijsong
added a commit
to kakao/varlog
that referenced
this pull request
Aug 9, 2022
* *: add topic Added topic to Varlog. * proto: add Topic into proto (#479) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go * Update internal/metadata_repository/storage.go Co-authored-by: jun.song(송인준)/kakao <[email protected]> * fix code * fix code Co-authored-by: pharrell.jang <[email protected]> Co-authored-by: jun.song(송인준)/kakao <[email protected]> * *: use int32 for storage node id and log stream id (#481) Changed type of `types.StorageNodeID` and `types.LogStreamID` from uint32 to int32. Resolves [#VARLOG-548](VARLOG-548). * topic: managemant topic (#485) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * wip * management topic * add test for register topic * add test for unregister topic * fmt * fix code * fix test * fix code * fix code Co-authored-by: pharrell.jang <[email protected]> * sn: remove redundant types for replica (#483) There were redundant types to represent replica: - `pkg/logc/StorageNode` - `proto/snpb/Replica` - `proto/snpb/AppendRequest_BackupNode` This patch removes those and uses `proto/varlogpb/StorageNode` and types that wrap it. Resolves [#VARLOG-546](VARLOG-546). * sn: add topic id to log i/o (#486) This patch adds `TopicID` to the methods of Log I/O interface. It doesn't contain any meaningful implementations about `TopicID`. Types that now have `TopicID` are follows: - `internal/storagenode/logio.ReadWriter` - `internal/storagenode/reportcommitter/reportcommitter.Getter` - `proto/snpb.AppendRequest`, `proto/snpb.AppendResponse`, `proto/snpb.ReadRequest`, `proto/snpb.SubscribeRequest`, `proto/snpb.TrimRequest` - `proto/varlogpb.Replica` Resolves [#VARLOG-542](VARLOG-542). * it: fix flaky test - TestVarlogSubscribeWithAddLS (#487) The `TestVarlogSubscribeWithAddLS` created a goroutine adding new LogStream while appending log entries. It however did not manage the life cycle of the goroutine resulting in several issues: - use closed connection to append logs - wait for commit messages from MR indefinitely since the MR is already closed This patch simply adds `sync.WaitGroup` to the test to avoid the above issues. Resolves [#VARLOG-569](VARLOG-569). * proto: removing unnecessary fields from messages (#488) This patch removes unnecessary fields generated automatically from proto messages. These lines are like follows: ``` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` ``` They are something related to either optimizations or compatibility that are generated by the gogoproto. However, they come to small overhead in many cases. (See cockroachdb/cockroach#38404 and etcd-io/etcd@e51c697#diff-76a35072df72591a656e69cab6f6fa99aa386fd5ace35c9042851eb324ec16b5). This change adds the following options to every proto definition file: ``` option (gogoproto.goproto_unkeyed_all) = false; option (gogoproto.goproto_unrecognized_all) = false; option (gogoproto.goproto_sizecache_all) = false; ``` Resolves [#VARLOG-557](VARLOG-557). * *: dedup LogEntry types (#489) - Remove `pkg/types.LogEntry`, then use `proto/varlogpb.LogEntry`. Resolves [#VARLOG-558](VARLOG-558). * vendor: bump Pebble (#490) Resolves #VARLOG-556 * sn: rename AddLogStream RPC (#491) In this patch, the RPC `AddLogStream` renames to `AddLogStreamReplica` to clarify its behavior. The RPC `AddLogStreamReplica` adds a new replica of the given LogStream to the StorageNode. Resolves [#VARLOG-568](VARLOG-568). * topic: apply Topic into client (#493) * logio: apply topic * add topic test * fix TestMRTopicLastHighWatermark * fix managing log stream status in vms Resolves #VARLOG-559 * all: update golang 1.17.0 (#492) Resolves [#VARLOG-555](VARLOG-555). * all: fix code style (#494) This patch just fixes code styles. Resolves [#VARLOG-572](VARLOG-572). * build: use predefined protoc (#496) Resolves [#VARLOG-563](VARLOG-563). * sn,topic: checking topic id while handling RPCs (#495) This patch adds a feature that the StorageNode checks if the topic id is valid or not. To support this functionality it adds a topicID to a parameter of many functions. The executor does not care about the topicID, rather it will be considered by StorageNode. To do that, StorageNode maintains the executors by using the executorsmap keyed by logStreamTopicID. The logStreamTopicID is a packed type of the LogStreamID and TopicID. Resolves [#VARLOG-542](VARLOG-542). * lint: fix code style (#497) This is a follow-up PR for VARLOG-572. Resolves [#VARLOG-572](VARLOG-572). Co-authored-by: pharrell.jang(장형근)/kakao <[email protected]> Co-authored-by: pharrell.jang <[email protected]>
ijsong
added a commit
to kakao/varlog
that referenced
this pull request
Aug 9, 2022
* *: add topic Added topic to Varlog. * proto: add Topic into proto (#479) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go * Update internal/metadata_repository/storage.go Co-authored-by: Injun Song <[email protected]> * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Injun Song <[email protected]> * *: use int32 for storage node id and log stream id (#481) Changed type of `types.StorageNodeID` and `types.LogStreamID` from uint32 to int32. Resolves [#VARLOG-548](VARLOG-548). * topic: managemant topic (#485) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * wip * management topic * add test for register topic * add test for unregister topic * fmt * fix code * fix test * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> * sn: remove redundant types for replica (#483) There were redundant types to represent replica: - `pkg/logc/StorageNode` - `proto/snpb/Replica` - `proto/snpb/AppendRequest_BackupNode` This patch removes those and uses `proto/varlogpb/StorageNode` and types that wrap it. Resolves [#VARLOG-546](VARLOG-546). * sn: add topic id to log i/o (#486) This patch adds `TopicID` to the methods of Log I/O interface. It doesn't contain any meaningful implementations about `TopicID`. Types that now have `TopicID` are follows: - `internal/storagenode/logio.ReadWriter` - `internal/storagenode/reportcommitter/reportcommitter.Getter` - `proto/snpb.AppendRequest`, `proto/snpb.AppendResponse`, `proto/snpb.ReadRequest`, `proto/snpb.SubscribeRequest`, `proto/snpb.TrimRequest` - `proto/varlogpb.Replica` Resolves [#VARLOG-542](VARLOG-542). * it: fix flaky test - TestVarlogSubscribeWithAddLS (#487) The `TestVarlogSubscribeWithAddLS` created a goroutine adding new LogStream while appending log entries. It however did not manage the life cycle of the goroutine resulting in several issues: - use closed connection to append logs - wait for commit messages from MR indefinitely since the MR is already closed This patch simply adds `sync.WaitGroup` to the test to avoid the above issues. Resolves [#VARLOG-569](VARLOG-569). * proto: removing unnecessary fields from messages (#488) This patch removes unnecessary fields generated automatically from proto messages. These lines are like follows: ``` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` ``` They are something related to either optimizations or compatibility that are generated by the gogoproto. However, they come to small overhead in many cases. (See cockroachdb/cockroach#38404 and etcd-io/etcd@e51c697#diff-76a35072df72591a656e69cab6f6fa99aa386fd5ace35c9042851eb324ec16b5). This change adds the following options to every proto definition file: ``` option (gogoproto.goproto_unkeyed_all) = false; option (gogoproto.goproto_unrecognized_all) = false; option (gogoproto.goproto_sizecache_all) = false; ``` Resolves [#VARLOG-557](VARLOG-557). * *: dedup LogEntry types (#489) - Remove `pkg/types.LogEntry`, then use `proto/varlogpb.LogEntry`. Resolves [#VARLOG-558](VARLOG-558). * vendor: bump Pebble (#490) Resolves #VARLOG-556 * sn: rename AddLogStream RPC (#491) In this patch, the RPC `AddLogStream` renames to `AddLogStreamReplica` to clarify its behavior. The RPC `AddLogStreamReplica` adds a new replica of the given LogStream to the StorageNode. Resolves [#VARLOG-568](VARLOG-568). * topic: apply Topic into client (#493) * logio: apply topic * add topic test * fix TestMRTopicLastHighWatermark * fix managing log stream status in vms Resolves #VARLOG-559 * all: update golang 1.17.0 (#492) Resolves [#VARLOG-555](VARLOG-555). * all: fix code style (#494) This patch just fixes code styles. Resolves [#VARLOG-572](VARLOG-572). * build: use predefined protoc (#496) Resolves [#VARLOG-563](VARLOG-563). * sn,topic: checking topic id while handling RPCs (#495) This patch adds a feature that the StorageNode checks if the topic id is valid or not. To support this functionality it adds a topicID to a parameter of many functions. The executor does not care about the topicID, rather it will be considered by StorageNode. To do that, StorageNode maintains the executors by using the executorsmap keyed by logStreamTopicID. The logStreamTopicID is a packed type of the LogStreamID and TopicID. Resolves [#VARLOG-542](VARLOG-542). * lint: fix code style (#497) This is a follow-up PR for VARLOG-572. Resolves [#VARLOG-572](VARLOG-572). Co-authored-by: pharrell.jang(장형근)/kakao <[email protected]> Co-authored-by: Hyungkeun Jang <[email protected]>
ijsong
added a commit
to kakao/varlog
that referenced
this pull request
Aug 9, 2022
* *: add topic Added topic to Varlog. * proto: add Topic into proto (#479) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go * Update internal/metadata_repository/storage.go Co-authored-by: Injun Song <[email protected]> * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Injun Song <[email protected]> * *: use int32 for storage node id and log stream id (#481) Changed type of `types.StorageNodeID` and `types.LogStreamID` from uint32 to int32. Resolves [#VARLOG-548](VARLOG-548). * topic: managemant topic (#485) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * wip * management topic * add test for register topic * add test for unregister topic * fmt * fix code * fix test * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> * sn: remove redundant types for replica (#483) There were redundant types to represent replica: - `pkg/logc/StorageNode` - `proto/snpb/Replica` - `proto/snpb/AppendRequest_BackupNode` This patch removes those and uses `proto/varlogpb/StorageNode` and types that wrap it. Resolves [#VARLOG-546](VARLOG-546). * sn: add topic id to log i/o (#486) This patch adds `TopicID` to the methods of Log I/O interface. It doesn't contain any meaningful implementations about `TopicID`. Types that now have `TopicID` are follows: - `internal/storagenode/logio.ReadWriter` - `internal/storagenode/reportcommitter/reportcommitter.Getter` - `proto/snpb.AppendRequest`, `proto/snpb.AppendResponse`, `proto/snpb.ReadRequest`, `proto/snpb.SubscribeRequest`, `proto/snpb.TrimRequest` - `proto/varlogpb.Replica` Resolves [#VARLOG-542](VARLOG-542). * it: fix flaky test - TestVarlogSubscribeWithAddLS (#487) The `TestVarlogSubscribeWithAddLS` created a goroutine adding new LogStream while appending log entries. It however did not manage the life cycle of the goroutine resulting in several issues: - use closed connection to append logs - wait for commit messages from MR indefinitely since the MR is already closed This patch simply adds `sync.WaitGroup` to the test to avoid the above issues. Resolves [#VARLOG-569](VARLOG-569). * proto: removing unnecessary fields from messages (#488) This patch removes unnecessary fields generated automatically from proto messages. These lines are like follows: ``` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` ``` They are something related to either optimizations or compatibility that are generated by the gogoproto. However, they come to small overhead in many cases. (See cockroachdb/cockroach#38404 and etcd-io/etcd@e51c697#diff-76a35072df72591a656e69cab6f6fa99aa386fd5ace35c9042851eb324ec16b5). This change adds the following options to every proto definition file: ``` option (gogoproto.goproto_unkeyed_all) = false; option (gogoproto.goproto_unrecognized_all) = false; option (gogoproto.goproto_sizecache_all) = false; ``` Resolves [#VARLOG-557](VARLOG-557). * *: dedup LogEntry types (#489) - Remove `pkg/types.LogEntry`, then use `proto/varlogpb.LogEntry`. Resolves [#VARLOG-558](VARLOG-558). * vendor: bump Pebble (#490) Resolves #VARLOG-556 * sn: rename AddLogStream RPC (#491) In this patch, the RPC `AddLogStream` renames to `AddLogStreamReplica` to clarify its behavior. The RPC `AddLogStreamReplica` adds a new replica of the given LogStream to the StorageNode. Resolves [#VARLOG-568](VARLOG-568). * topic: apply Topic into client (#493) * logio: apply topic * add topic test * fix TestMRTopicLastHighWatermark * fix managing log stream status in vms Resolves #VARLOG-559 * all: update golang 1.17.0 (#492) Resolves [#VARLOG-555](VARLOG-555). * all: fix code style (#494) This patch just fixes code styles. Resolves [#VARLOG-572](VARLOG-572). * build: use predefined protoc (#496) Resolves [#VARLOG-563](VARLOG-563). * sn,topic: checking topic id while handling RPCs (#495) This patch adds a feature that the StorageNode checks if the topic id is valid or not. To support this functionality it adds a topicID to a parameter of many functions. The executor does not care about the topicID, rather it will be considered by StorageNode. To do that, StorageNode maintains the executors by using the executorsmap keyed by logStreamTopicID. The logStreamTopicID is a packed type of the LogStreamID and TopicID. Resolves [#VARLOG-542](VARLOG-542). * lint: fix code style (#497) This is a follow-up PR for VARLOG-572. Resolves [#VARLOG-572](VARLOG-572). Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Hyungkeun Jang <[email protected]>
ijsong
added a commit
to kakao/varlog
that referenced
this pull request
Aug 9, 2022
* *: add topic Added topic to Varlog. * proto: add Topic into proto (#479) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go * Update internal/metadata_repository/storage.go Co-authored-by: Injun Song <[email protected]> * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Injun Song <[email protected]> * *: use int32 for storage node id and log stream id (#481) Changed type of `types.StorageNodeID` and `types.LogStreamID` from uint32 to int32. Resolves [#VARLOG-548](VARLOG-548). * topic: managemant topic (#485) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * wip * management topic * add test for register topic * add test for unregister topic * fmt * fix code * fix test * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> * sn: remove redundant types for replica (#483) There were redundant types to represent replica: - `pkg/logc/StorageNode` - `proto/snpb/Replica` - `proto/snpb/AppendRequest_BackupNode` This patch removes those and uses `proto/varlogpb/StorageNode` and types that wrap it. Resolves [#VARLOG-546](VARLOG-546). * sn: add topic id to log i/o (#486) This patch adds `TopicID` to the methods of Log I/O interface. It doesn't contain any meaningful implementations about `TopicID`. Types that now have `TopicID` are follows: - `internal/storagenode/logio.ReadWriter` - `internal/storagenode/reportcommitter/reportcommitter.Getter` - `proto/snpb.AppendRequest`, `proto/snpb.AppendResponse`, `proto/snpb.ReadRequest`, `proto/snpb.SubscribeRequest`, `proto/snpb.TrimRequest` - `proto/varlogpb.Replica` Resolves [#VARLOG-542](VARLOG-542). * it: fix flaky test - TestVarlogSubscribeWithAddLS (#487) The `TestVarlogSubscribeWithAddLS` created a goroutine adding new LogStream while appending log entries. It however did not manage the life cycle of the goroutine resulting in several issues: - use closed connection to append logs - wait for commit messages from MR indefinitely since the MR is already closed This patch simply adds `sync.WaitGroup` to the test to avoid the above issues. Resolves [#VARLOG-569](VARLOG-569). * proto: removing unnecessary fields from messages (#488) This patch removes unnecessary fields generated automatically from proto messages. These lines are like follows: ``` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` ``` They are something related to either optimizations or compatibility that are generated by the gogoproto. However, they come to small overhead in many cases. (See cockroachdb/cockroach#38404 and etcd-io/etcd@e51c697#diff-76a35072df72591a656e69cab6f6fa99aa386fd5ace35c9042851eb324ec16b5). This change adds the following options to every proto definition file: ``` option (gogoproto.goproto_unkeyed_all) = false; option (gogoproto.goproto_unrecognized_all) = false; option (gogoproto.goproto_sizecache_all) = false; ``` Resolves [#VARLOG-557](VARLOG-557). * *: dedup LogEntry types (#489) - Remove `pkg/types.LogEntry`, then use `proto/varlogpb.LogEntry`. Resolves [#VARLOG-558](VARLOG-558). * vendor: bump Pebble (#490) Resolves #VARLOG-556 * sn: rename AddLogStream RPC (#491) In this patch, the RPC `AddLogStream` renames to `AddLogStreamReplica` to clarify its behavior. The RPC `AddLogStreamReplica` adds a new replica of the given LogStream to the StorageNode. Resolves [#VARLOG-568](VARLOG-568). * topic: apply Topic into client (#493) * logio: apply topic * add topic test * fix TestMRTopicLastHighWatermark * fix managing log stream status in vms Resolves #VARLOG-559 * all: update golang 1.17.0 (#492) Resolves [#VARLOG-555](VARLOG-555). * all: fix code style (#494) This patch just fixes code styles. Resolves [#VARLOG-572](VARLOG-572). * build: use predefined protoc (#496) Resolves [#VARLOG-563](VARLOG-563). * sn,topic: checking topic id while handling RPCs (#495) This patch adds a feature that the StorageNode checks if the topic id is valid or not. To support this functionality it adds a topicID to a parameter of many functions. The executor does not care about the topicID, rather it will be considered by StorageNode. To do that, StorageNode maintains the executors by using the executorsmap keyed by logStreamTopicID. The logStreamTopicID is a packed type of the LogStreamID and TopicID. Resolves [#VARLOG-542](VARLOG-542). * lint: fix code style (#497) This is a follow-up PR for VARLOG-572. Resolves [#VARLOG-572](VARLOG-572). Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Hyungkeun Jang <[email protected]>
ijsong
added a commit
to kakao/varlog
that referenced
this pull request
Aug 9, 2022
* *: add topic Added topic to Varlog. * proto: add Topic into proto (#479) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go * Update internal/metadata_repository/storage.go Co-authored-by: Injun Song <[email protected]> * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Injun Song <[email protected]> * *: use int32 for storage node id and log stream id (#481) Changed type of `types.StorageNodeID` and `types.LogStreamID` from uint32 to int32. Resolves [#VARLOG-548](VARLOG-548). * topic: managemant topic (#485) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * wip * management topic * add test for register topic * add test for unregister topic * fmt * fix code * fix test * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> * sn: remove redundant types for replica (#483) There were redundant types to represent replica: - `pkg/logc/StorageNode` - `proto/snpb/Replica` - `proto/snpb/AppendRequest_BackupNode` This patch removes those and uses `proto/varlogpb/StorageNode` and types that wrap it. Resolves [#VARLOG-546](VARLOG-546). * sn: add topic id to log i/o (#486) This patch adds `TopicID` to the methods of Log I/O interface. It doesn't contain any meaningful implementations about `TopicID`. Types that now have `TopicID` are follows: - `internal/storagenode/logio.ReadWriter` - `internal/storagenode/reportcommitter/reportcommitter.Getter` - `proto/snpb.AppendRequest`, `proto/snpb.AppendResponse`, `proto/snpb.ReadRequest`, `proto/snpb.SubscribeRequest`, `proto/snpb.TrimRequest` - `proto/varlogpb.Replica` Resolves [#VARLOG-542](VARLOG-542). * it: fix flaky test - TestVarlogSubscribeWithAddLS (#487) The `TestVarlogSubscribeWithAddLS` created a goroutine adding new LogStream while appending log entries. It however did not manage the life cycle of the goroutine resulting in several issues: - use closed connection to append logs - wait for commit messages from MR indefinitely since the MR is already closed This patch simply adds `sync.WaitGroup` to the test to avoid the above issues. Resolves [#VARLOG-569](VARLOG-569). * proto: removing unnecessary fields from messages (#488) This patch removes unnecessary fields generated automatically from proto messages. These lines are like follows: ``` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` ``` They are something related to either optimizations or compatibility that are generated by the gogoproto. However, they come to small overhead in many cases. (See cockroachdb/cockroach#38404 and etcd-io/etcd@e51c697#diff-76a35072df72591a656e69cab6f6fa99aa386fd5ace35c9042851eb324ec16b5). This change adds the following options to every proto definition file: ``` option (gogoproto.goproto_unkeyed_all) = false; option (gogoproto.goproto_unrecognized_all) = false; option (gogoproto.goproto_sizecache_all) = false; ``` Resolves [#VARLOG-557](VARLOG-557). * *: dedup LogEntry types (#489) - Remove `pkg/types.LogEntry`, then use `proto/varlogpb.LogEntry`. Resolves [#VARLOG-558](VARLOG-558). * vendor: bump Pebble (#490) Resolves #VARLOG-556 * sn: rename AddLogStream RPC (#491) In this patch, the RPC `AddLogStream` renames to `AddLogStreamReplica` to clarify its behavior. The RPC `AddLogStreamReplica` adds a new replica of the given LogStream to the StorageNode. Resolves [#VARLOG-568](VARLOG-568). * topic: apply Topic into client (#493) * logio: apply topic * add topic test * fix TestMRTopicLastHighWatermark * fix managing log stream status in vms Resolves #VARLOG-559 * all: update golang 1.17.0 (#492) Resolves [#VARLOG-555](VARLOG-555). * all: fix code style (#494) This patch just fixes code styles. Resolves [#VARLOG-572](VARLOG-572). * build: use predefined protoc (#496) Resolves [#VARLOG-563](VARLOG-563). * sn,topic: checking topic id while handling RPCs (#495) This patch adds a feature that the StorageNode checks if the topic id is valid or not. To support this functionality it adds a topicID to a parameter of many functions. The executor does not care about the topicID, rather it will be considered by StorageNode. To do that, StorageNode maintains the executors by using the executorsmap keyed by logStreamTopicID. The logStreamTopicID is a packed type of the LogStreamID and TopicID. Resolves [#VARLOG-542](VARLOG-542). * lint: fix code style (#497) This is a follow-up PR for VARLOG-572. Resolves [#VARLOG-572](VARLOG-572). Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Hyungkeun Jang <[email protected]>
ijsong
added a commit
to kakao/varlog
that referenced
this pull request
Aug 9, 2022
* *: add topic Added topic to Varlog. * proto: add Topic into proto (#479) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go * Update internal/metadata_repository/storage.go Co-authored-by: Injun Song <[email protected]> * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Injun Song <[email protected]> * *: use int32 for storage node id and log stream id (#481) Changed type of `types.StorageNodeID` and `types.LogStreamID` from uint32 to int32. Resolves [#VARLOG-548](VARLOG-548). * topic: managemant topic (#485) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * wip * management topic * add test for register topic * add test for unregister topic * fmt * fix code * fix test * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> * sn: remove redundant types for replica (#483) There were redundant types to represent replica: - `pkg/logc/StorageNode` - `proto/snpb/Replica` - `proto/snpb/AppendRequest_BackupNode` This patch removes those and uses `proto/varlogpb/StorageNode` and types that wrap it. Resolves [#VARLOG-546](VARLOG-546). * sn: add topic id to log i/o (#486) This patch adds `TopicID` to the methods of Log I/O interface. It doesn't contain any meaningful implementations about `TopicID`. Types that now have `TopicID` are follows: - `internal/storagenode/logio.ReadWriter` - `internal/storagenode/reportcommitter/reportcommitter.Getter` - `proto/snpb.AppendRequest`, `proto/snpb.AppendResponse`, `proto/snpb.ReadRequest`, `proto/snpb.SubscribeRequest`, `proto/snpb.TrimRequest` - `proto/varlogpb.Replica` Resolves [#VARLOG-542](VARLOG-542). * it: fix flaky test - TestVarlogSubscribeWithAddLS (#487) The `TestVarlogSubscribeWithAddLS` created a goroutine adding new LogStream while appending log entries. It however did not manage the life cycle of the goroutine resulting in several issues: - use closed connection to append logs - wait for commit messages from MR indefinitely since the MR is already closed This patch simply adds `sync.WaitGroup` to the test to avoid the above issues. Resolves [#VARLOG-569](VARLOG-569). * proto: removing unnecessary fields from messages (#488) This patch removes unnecessary fields generated automatically from proto messages. These lines are like follows: ``` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` ``` They are something related to either optimizations or compatibility that are generated by the gogoproto. However, they come to small overhead in many cases. (See cockroachdb/cockroach#38404 and etcd-io/etcd@e51c697#diff-76a35072df72591a656e69cab6f6fa99aa386fd5ace35c9042851eb324ec16b5). This change adds the following options to every proto definition file: ``` option (gogoproto.goproto_unkeyed_all) = false; option (gogoproto.goproto_unrecognized_all) = false; option (gogoproto.goproto_sizecache_all) = false; ``` Resolves [#VARLOG-557](VARLOG-557). * *: dedup LogEntry types (#489) - Remove `pkg/types.LogEntry`, then use `proto/varlogpb.LogEntry`. Resolves [#VARLOG-558](VARLOG-558). * vendor: bump Pebble (#490) Resolves #VARLOG-556 * sn: rename AddLogStream RPC (#491) In this patch, the RPC `AddLogStream` renames to `AddLogStreamReplica` to clarify its behavior. The RPC `AddLogStreamReplica` adds a new replica of the given LogStream to the StorageNode. Resolves [#VARLOG-568](VARLOG-568). * topic: apply Topic into client (#493) * logio: apply topic * add topic test * fix TestMRTopicLastHighWatermark * fix managing log stream status in vms Resolves #VARLOG-559 * all: update golang 1.17.0 (#492) Resolves [#VARLOG-555](VARLOG-555). * all: fix code style (#494) This patch just fixes code styles. Resolves [#VARLOG-572](VARLOG-572). * build: use predefined protoc (#496) Resolves [#VARLOG-563](VARLOG-563). * sn,topic: checking topic id while handling RPCs (#495) This patch adds a feature that the StorageNode checks if the topic id is valid or not. To support this functionality it adds a topicID to a parameter of many functions. The executor does not care about the topicID, rather it will be considered by StorageNode. To do that, StorageNode maintains the executors by using the executorsmap keyed by logStreamTopicID. The logStreamTopicID is a packed type of the LogStreamID and TopicID. Resolves [#VARLOG-542](VARLOG-542). * lint: fix code style (#497) This is a follow-up PR for VARLOG-572. Resolves [#VARLOG-572](VARLOG-572). Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Hyungkeun Jang <[email protected]>
ijsong
added a commit
to kakao/varlog
that referenced
this pull request
Aug 9, 2022
* *: add topic Added topic to Varlog. * proto: add Topic into proto (#479) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go * Update internal/metadata_repository/storage.go Co-authored-by: Injun Song <[email protected]> * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Injun Song <[email protected]> * *: use int32 for storage node id and log stream id (#481) Changed type of `types.StorageNodeID` and `types.LogStreamID` from uint32 to int32. Resolves [#VARLOG-548](VARLOG-548). * topic: managemant topic (#485) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * wip * management topic * add test for register topic * add test for unregister topic * fmt * fix code * fix test * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> * sn: remove redundant types for replica (#483) There were redundant types to represent replica: - `pkg/logc/StorageNode` - `proto/snpb/Replica` - `proto/snpb/AppendRequest_BackupNode` This patch removes those and uses `proto/varlogpb/StorageNode` and types that wrap it. Resolves [#VARLOG-546](VARLOG-546). * sn: add topic id to log i/o (#486) This patch adds `TopicID` to the methods of Log I/O interface. It doesn't contain any meaningful implementations about `TopicID`. Types that now have `TopicID` are follows: - `internal/storagenode/logio.ReadWriter` - `internal/storagenode/reportcommitter/reportcommitter.Getter` - `proto/snpb.AppendRequest`, `proto/snpb.AppendResponse`, `proto/snpb.ReadRequest`, `proto/snpb.SubscribeRequest`, `proto/snpb.TrimRequest` - `proto/varlogpb.Replica` Resolves [#VARLOG-542](VARLOG-542). * it: fix flaky test - TestVarlogSubscribeWithAddLS (#487) The `TestVarlogSubscribeWithAddLS` created a goroutine adding new LogStream while appending log entries. It however did not manage the life cycle of the goroutine resulting in several issues: - use closed connection to append logs - wait for commit messages from MR indefinitely since the MR is already closed This patch simply adds `sync.WaitGroup` to the test to avoid the above issues. Resolves [#VARLOG-569](VARLOG-569). * proto: removing unnecessary fields from messages (#488) This patch removes unnecessary fields generated automatically from proto messages. These lines are like follows: ``` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` ``` They are something related to either optimizations or compatibility that are generated by the gogoproto. However, they come to small overhead in many cases. (See cockroachdb/cockroach#38404 and etcd-io/etcd@e51c697#diff-76a35072df72591a656e69cab6f6fa99aa386fd5ace35c9042851eb324ec16b5). This change adds the following options to every proto definition file: ``` option (gogoproto.goproto_unkeyed_all) = false; option (gogoproto.goproto_unrecognized_all) = false; option (gogoproto.goproto_sizecache_all) = false; ``` Resolves [#VARLOG-557](VARLOG-557). * *: dedup LogEntry types (#489) - Remove `pkg/types.LogEntry`, then use `proto/varlogpb.LogEntry`. Resolves [#VARLOG-558](VARLOG-558). * vendor: bump Pebble (#490) Resolves #VARLOG-556 * sn: rename AddLogStream RPC (#491) In this patch, the RPC `AddLogStream` renames to `AddLogStreamReplica` to clarify its behavior. The RPC `AddLogStreamReplica` adds a new replica of the given LogStream to the StorageNode. Resolves [#VARLOG-568](VARLOG-568). * topic: apply Topic into client (#493) * logio: apply topic * add topic test * fix TestMRTopicLastHighWatermark * fix managing log stream status in vms Resolves #VARLOG-559 * all: update golang 1.17.0 (#492) Resolves [#VARLOG-555](VARLOG-555). * all: fix code style (#494) This patch just fixes code styles. Resolves [#VARLOG-572](VARLOG-572). * build: use predefined protoc (#496) Resolves [#VARLOG-563](VARLOG-563). * sn,topic: checking topic id while handling RPCs (#495) This patch adds a feature that the StorageNode checks if the topic id is valid or not. To support this functionality it adds a topicID to a parameter of many functions. The executor does not care about the topicID, rather it will be considered by StorageNode. To do that, StorageNode maintains the executors by using the executorsmap keyed by logStreamTopicID. The logStreamTopicID is a packed type of the LogStreamID and TopicID. Resolves [#VARLOG-542](VARLOG-542). * lint: fix code style (#497) This is a follow-up PR for VARLOG-572. Resolves [#VARLOG-572](VARLOG-572). Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Hyungkeun Jang <[email protected]>
ijsong
added a commit
to kakao/varlog
that referenced
this pull request
Aug 9, 2022
* *: add topic Added topic to Varlog. * proto: add Topic into proto (#479) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go * Update internal/metadata_repository/storage.go Co-authored-by: Injun Song <[email protected]> * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Injun Song <[email protected]> * *: use int32 for storage node id and log stream id (#481) Changed type of `types.StorageNodeID` and `types.LogStreamID` from uint32 to int32. Resolves [#VARLOG-548](VARLOG-548). * topic: managemant topic (#485) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * wip * management topic * add test for register topic * add test for unregister topic * fmt * fix code * fix test * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> * sn: remove redundant types for replica (#483) There were redundant types to represent replica: - `pkg/logc/StorageNode` - `proto/snpb/Replica` - `proto/snpb/AppendRequest_BackupNode` This patch removes those and uses `proto/varlogpb/StorageNode` and types that wrap it. Resolves [#VARLOG-546](VARLOG-546). * sn: add topic id to log i/o (#486) This patch adds `TopicID` to the methods of Log I/O interface. It doesn't contain any meaningful implementations about `TopicID`. Types that now have `TopicID` are follows: - `internal/storagenode/logio.ReadWriter` - `internal/storagenode/reportcommitter/reportcommitter.Getter` - `proto/snpb.AppendRequest`, `proto/snpb.AppendResponse`, `proto/snpb.ReadRequest`, `proto/snpb.SubscribeRequest`, `proto/snpb.TrimRequest` - `proto/varlogpb.Replica` Resolves [#VARLOG-542](VARLOG-542). * it: fix flaky test - TestVarlogSubscribeWithAddLS (#487) The `TestVarlogSubscribeWithAddLS` created a goroutine adding new LogStream while appending log entries. It however did not manage the life cycle of the goroutine resulting in several issues: - use closed connection to append logs - wait for commit messages from MR indefinitely since the MR is already closed This patch simply adds `sync.WaitGroup` to the test to avoid the above issues. Resolves [#VARLOG-569](VARLOG-569). * proto: removing unnecessary fields from messages (#488) This patch removes unnecessary fields generated automatically from proto messages. These lines are like follows: ``` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` ``` They are something related to either optimizations or compatibility that are generated by the gogoproto. However, they come to small overhead in many cases. (See cockroachdb/cockroach#38404 and etcd-io/etcd@e51c697#diff-76a35072df72591a656e69cab6f6fa99aa386fd5ace35c9042851eb324ec16b5). This change adds the following options to every proto definition file: ``` option (gogoproto.goproto_unkeyed_all) = false; option (gogoproto.goproto_unrecognized_all) = false; option (gogoproto.goproto_sizecache_all) = false; ``` Resolves [#VARLOG-557](VARLOG-557). * *: dedup LogEntry types (#489) - Remove `pkg/types.LogEntry`, then use `proto/varlogpb.LogEntry`. Resolves [#VARLOG-558](VARLOG-558). * vendor: bump Pebble (#490) Resolves #VARLOG-556 * sn: rename AddLogStream RPC (#491) In this patch, the RPC `AddLogStream` renames to `AddLogStreamReplica` to clarify its behavior. The RPC `AddLogStreamReplica` adds a new replica of the given LogStream to the StorageNode. Resolves [#VARLOG-568](VARLOG-568). * topic: apply Topic into client (#493) * logio: apply topic * add topic test * fix TestMRTopicLastHighWatermark * fix managing log stream status in vms Resolves #VARLOG-559 * all: update golang 1.17.0 (#492) Resolves [#VARLOG-555](VARLOG-555). * all: fix code style (#494) This patch just fixes code styles. Resolves [#VARLOG-572](VARLOG-572). * build: use predefined protoc (#496) Resolves [#VARLOG-563](VARLOG-563). * sn,topic: checking topic id while handling RPCs (#495) This patch adds a feature that the StorageNode checks if the topic id is valid or not. To support this functionality it adds a topicID to a parameter of many functions. The executor does not care about the topicID, rather it will be considered by StorageNode. To do that, StorageNode maintains the executors by using the executorsmap keyed by logStreamTopicID. The logStreamTopicID is a packed type of the LogStreamID and TopicID. Resolves [#VARLOG-542](VARLOG-542). * lint: fix code style (#497) This is a follow-up PR for VARLOG-572. Resolves [#VARLOG-572](VARLOG-572). Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Hyungkeun Jang <[email protected]>
ijsong
added a commit
to kakao/varlog
that referenced
this pull request
Aug 10, 2022
* *: add topic Added topic to Varlog. * proto: add Topic into proto (#479) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go * Update internal/metadata_repository/storage.go Co-authored-by: Injun Song <[email protected]> * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Injun Song <[email protected]> * *: use int32 for storage node id and log stream id (#481) Changed type of `types.StorageNodeID` and `types.LogStreamID` from uint32 to int32. Resolves [#VARLOG-548](VARLOG-548). * topic: managemant topic (#485) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * wip * management topic * add test for register topic * add test for unregister topic * fmt * fix code * fix test * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> * sn: remove redundant types for replica (#483) There were redundant types to represent replica: - `pkg/logc/StorageNode` - `proto/snpb/Replica` - `proto/snpb/AppendRequest_BackupNode` This patch removes those and uses `proto/varlogpb/StorageNode` and types that wrap it. Resolves [#VARLOG-546](VARLOG-546). * sn: add topic id to log i/o (#486) This patch adds `TopicID` to the methods of Log I/O interface. It doesn't contain any meaningful implementations about `TopicID`. Types that now have `TopicID` are follows: - `internal/storagenode/logio.ReadWriter` - `internal/storagenode/reportcommitter/reportcommitter.Getter` - `proto/snpb.AppendRequest`, `proto/snpb.AppendResponse`, `proto/snpb.ReadRequest`, `proto/snpb.SubscribeRequest`, `proto/snpb.TrimRequest` - `proto/varlogpb.Replica` Resolves [#VARLOG-542](VARLOG-542). * it: fix flaky test - TestVarlogSubscribeWithAddLS (#487) The `TestVarlogSubscribeWithAddLS` created a goroutine adding new LogStream while appending log entries. It however did not manage the life cycle of the goroutine resulting in several issues: - use closed connection to append logs - wait for commit messages from MR indefinitely since the MR is already closed This patch simply adds `sync.WaitGroup` to the test to avoid the above issues. Resolves [#VARLOG-569](VARLOG-569). * proto: removing unnecessary fields from messages (#488) This patch removes unnecessary fields generated automatically from proto messages. These lines are like follows: ``` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` ``` They are something related to either optimizations or compatibility that are generated by the gogoproto. However, they come to small overhead in many cases. (See cockroachdb/cockroach#38404 and etcd-io/etcd@e51c697#diff-76a35072df72591a656e69cab6f6fa99aa386fd5ace35c9042851eb324ec16b5). This change adds the following options to every proto definition file: ``` option (gogoproto.goproto_unkeyed_all) = false; option (gogoproto.goproto_unrecognized_all) = false; option (gogoproto.goproto_sizecache_all) = false; ``` Resolves [#VARLOG-557](VARLOG-557). * *: dedup LogEntry types (#489) - Remove `pkg/types.LogEntry`, then use `proto/varlogpb.LogEntry`. Resolves [#VARLOG-558](VARLOG-558). * vendor: bump Pebble (#490) Resolves #VARLOG-556 * sn: rename AddLogStream RPC (#491) In this patch, the RPC `AddLogStream` renames to `AddLogStreamReplica` to clarify its behavior. The RPC `AddLogStreamReplica` adds a new replica of the given LogStream to the StorageNode. Resolves [#VARLOG-568](VARLOG-568). * topic: apply Topic into client (#493) * logio: apply topic * add topic test * fix TestMRTopicLastHighWatermark * fix managing log stream status in vms Resolves #VARLOG-559 * all: update golang 1.17.0 (#492) Resolves [#VARLOG-555](VARLOG-555). * all: fix code style (#494) This patch just fixes code styles. Resolves [#VARLOG-572](VARLOG-572). * build: use predefined protoc (#496) Resolves [#VARLOG-563](VARLOG-563). * sn,topic: checking topic id while handling RPCs (#495) This patch adds a feature that the StorageNode checks if the topic id is valid or not. To support this functionality it adds a topicID to a parameter of many functions. The executor does not care about the topicID, rather it will be considered by StorageNode. To do that, StorageNode maintains the executors by using the executorsmap keyed by logStreamTopicID. The logStreamTopicID is a packed type of the LogStreamID and TopicID. Resolves [#VARLOG-542](VARLOG-542). * lint: fix code style (#497) This is a follow-up PR for VARLOG-572. Resolves [#VARLOG-572](VARLOG-572). Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Hyungkeun Jang <[email protected]>
ijsong
added a commit
to kakao/varlog
that referenced
this pull request
Aug 10, 2022
* *: add topic Added topic to Varlog. * proto: add Topic into proto (#479) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go * Update internal/metadata_repository/storage.go Co-authored-by: Injun Song <[email protected]> * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Injun Song <[email protected]> * *: use int32 for storage node id and log stream id (#481) Changed type of `types.StorageNodeID` and `types.LogStreamID` from uint32 to int32. Resolves [#VARLOG-548](VARLOG-548). * topic: managemant topic (#485) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * wip * management topic * add test for register topic * add test for unregister topic * fmt * fix code * fix test * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> * sn: remove redundant types for replica (#483) There were redundant types to represent replica: - `pkg/logc/StorageNode` - `proto/snpb/Replica` - `proto/snpb/AppendRequest_BackupNode` This patch removes those and uses `proto/varlogpb/StorageNode` and types that wrap it. Resolves [#VARLOG-546](VARLOG-546). * sn: add topic id to log i/o (#486) This patch adds `TopicID` to the methods of Log I/O interface. It doesn't contain any meaningful implementations about `TopicID`. Types that now have `TopicID` are follows: - `internal/storagenode/logio.ReadWriter` - `internal/storagenode/reportcommitter/reportcommitter.Getter` - `proto/snpb.AppendRequest`, `proto/snpb.AppendResponse`, `proto/snpb.ReadRequest`, `proto/snpb.SubscribeRequest`, `proto/snpb.TrimRequest` - `proto/varlogpb.Replica` Resolves [#VARLOG-542](VARLOG-542). * it: fix flaky test - TestVarlogSubscribeWithAddLS (#487) The `TestVarlogSubscribeWithAddLS` created a goroutine adding new LogStream while appending log entries. It however did not manage the life cycle of the goroutine resulting in several issues: - use closed connection to append logs - wait for commit messages from MR indefinitely since the MR is already closed This patch simply adds `sync.WaitGroup` to the test to avoid the above issues. Resolves [#VARLOG-569](VARLOG-569). * proto: removing unnecessary fields from messages (#488) This patch removes unnecessary fields generated automatically from proto messages. These lines are like follows: ``` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` ``` They are something related to either optimizations or compatibility that are generated by the gogoproto. However, they come to small overhead in many cases. (See cockroachdb/cockroach#38404 and etcd-io/etcd@e51c697#diff-76a35072df72591a656e69cab6f6fa99aa386fd5ace35c9042851eb324ec16b5). This change adds the following options to every proto definition file: ``` option (gogoproto.goproto_unkeyed_all) = false; option (gogoproto.goproto_unrecognized_all) = false; option (gogoproto.goproto_sizecache_all) = false; ``` Resolves [#VARLOG-557](VARLOG-557). * *: dedup LogEntry types (#489) - Remove `pkg/types.LogEntry`, then use `proto/varlogpb.LogEntry`. Resolves [#VARLOG-558](VARLOG-558). * vendor: bump Pebble (#490) Resolves #VARLOG-556 * sn: rename AddLogStream RPC (#491) In this patch, the RPC `AddLogStream` renames to `AddLogStreamReplica` to clarify its behavior. The RPC `AddLogStreamReplica` adds a new replica of the given LogStream to the StorageNode. Resolves [#VARLOG-568](VARLOG-568). * topic: apply Topic into client (#493) * logio: apply topic * add topic test * fix TestMRTopicLastHighWatermark * fix managing log stream status in vms Resolves #VARLOG-559 * all: update golang 1.17.0 (#492) Resolves [#VARLOG-555](VARLOG-555). * all: fix code style (#494) This patch just fixes code styles. Resolves [#VARLOG-572](VARLOG-572). * build: use predefined protoc (#496) Resolves [#VARLOG-563](VARLOG-563). * sn,topic: checking topic id while handling RPCs (#495) This patch adds a feature that the StorageNode checks if the topic id is valid or not. To support this functionality it adds a topicID to a parameter of many functions. The executor does not care about the topicID, rather it will be considered by StorageNode. To do that, StorageNode maintains the executors by using the executorsmap keyed by logStreamTopicID. The logStreamTopicID is a packed type of the LogStreamID and TopicID. Resolves [#VARLOG-542](VARLOG-542). * lint: fix code style (#497) This is a follow-up PR for VARLOG-572. Resolves [#VARLOG-572](VARLOG-572). Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Hyungkeun Jang <[email protected]>
ijsong
added a commit
to kakao/varlog
that referenced
this pull request
Aug 10, 2022
* *: add topic Added topic to Varlog. * proto: add Topic into proto (#479) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go Co-authored-by: Injun Song <[email protected]> * Update internal/metadata_repository/raft_metadata_repository.go * Update internal/metadata_repository/storage.go Co-authored-by: Injun Song <[email protected]> * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Injun Song <[email protected]> * *: use int32 for storage node id and log stream id (#481) Changed type of `types.StorageNodeID` and `types.LogStreamID` from uint32 to int32. Resolves [#VARLOG-548](VARLOG-548). * topic: managemant topic (#485) * add topic * add topic into proto * wip * fix CommitContextOf * add highWatermark into report * wip * management topic * add test for register topic * add test for unregister topic * fmt * fix code * fix test * fix code * fix code Co-authored-by: Hyungkeun Jang <[email protected]> * sn: remove redundant types for replica (#483) There were redundant types to represent replica: - `pkg/logc/StorageNode` - `proto/snpb/Replica` - `proto/snpb/AppendRequest_BackupNode` This patch removes those and uses `proto/varlogpb/StorageNode` and types that wrap it. Resolves [#VARLOG-546](VARLOG-546). * sn: add topic id to log i/o (#486) This patch adds `TopicID` to the methods of Log I/O interface. It doesn't contain any meaningful implementations about `TopicID`. Types that now have `TopicID` are follows: - `internal/storagenode/logio.ReadWriter` - `internal/storagenode/reportcommitter/reportcommitter.Getter` - `proto/snpb.AppendRequest`, `proto/snpb.AppendResponse`, `proto/snpb.ReadRequest`, `proto/snpb.SubscribeRequest`, `proto/snpb.TrimRequest` - `proto/varlogpb.Replica` Resolves [#VARLOG-542](VARLOG-542). * it: fix flaky test - TestVarlogSubscribeWithAddLS (#487) The `TestVarlogSubscribeWithAddLS` created a goroutine adding new LogStream while appending log entries. It however did not manage the life cycle of the goroutine resulting in several issues: - use closed connection to append logs - wait for commit messages from MR indefinitely since the MR is already closed This patch simply adds `sync.WaitGroup` to the test to avoid the above issues. Resolves [#VARLOG-569](VARLOG-569). * proto: removing unnecessary fields from messages (#488) This patch removes unnecessary fields generated automatically from proto messages. These lines are like follows: ``` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` ``` They are something related to either optimizations or compatibility that are generated by the gogoproto. However, they come to small overhead in many cases. (See cockroachdb/cockroach#38404 and etcd-io/etcd@e51c697#diff-76a35072df72591a656e69cab6f6fa99aa386fd5ace35c9042851eb324ec16b5). This change adds the following options to every proto definition file: ``` option (gogoproto.goproto_unkeyed_all) = false; option (gogoproto.goproto_unrecognized_all) = false; option (gogoproto.goproto_sizecache_all) = false; ``` Resolves [#VARLOG-557](VARLOG-557). * *: dedup LogEntry types (#489) - Remove `pkg/types.LogEntry`, then use `proto/varlogpb.LogEntry`. Resolves [#VARLOG-558](VARLOG-558). * vendor: bump Pebble (#490) Resolves #VARLOG-556 * sn: rename AddLogStream RPC (#491) In this patch, the RPC `AddLogStream` renames to `AddLogStreamReplica` to clarify its behavior. The RPC `AddLogStreamReplica` adds a new replica of the given LogStream to the StorageNode. Resolves [#VARLOG-568](VARLOG-568). * topic: apply Topic into client (#493) * logio: apply topic * add topic test * fix TestMRTopicLastHighWatermark * fix managing log stream status in vms Resolves #VARLOG-559 * all: update golang 1.17.0 (#492) Resolves [#VARLOG-555](VARLOG-555). * all: fix code style (#494) This patch just fixes code styles. Resolves [#VARLOG-572](VARLOG-572). * build: use predefined protoc (#496) Resolves [#VARLOG-563](VARLOG-563). * sn,topic: checking topic id while handling RPCs (#495) This patch adds a feature that the StorageNode checks if the topic id is valid or not. To support this functionality it adds a topicID to a parameter of many functions. The executor does not care about the topicID, rather it will be considered by StorageNode. To do that, StorageNode maintains the executors by using the executorsmap keyed by logStreamTopicID. The logStreamTopicID is a packed type of the LogStreamID and TopicID. Resolves [#VARLOG-542](VARLOG-542). * lint: fix code style (#497) This is a follow-up PR for VARLOG-572. Resolves [#VARLOG-572](VARLOG-572). Co-authored-by: Hyungkeun Jang <[email protected]> Co-authored-by: Hyungkeun Jang <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #37706.
The PR removes the
XXX_NoUnkeyedLiteralandXXX_sizecachefields from generated protobufs.XXX_NoUnkeyedLiteralclaims to come with a small perf win for proto marshaling, but it looks like it's only used withproto.Marshaland not with theMarshalmethod (which CRDB code and gRPC code always use). Meanwhile, it has two moderate disadvantages:spanlatch.Manager, which is what tipped me off to this new field), it has a non-negligible cost.hlc.Timestamp) and switching to anEqualsmethod would come with a cost to code complexity and runtime performance.After removing
XXX_NoUnkeyedLiteral,XXX_sizecacheis now at the end of generated protos, it is now taking up space even though it is an empty struct: golang/go#9401. Since this is just a glorified lint which is already enforced bygo vet's “composites” check, it doesn't seem worth the cost.Benchmarks