You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yuqi Du (Code Review)" <ge...@cloudera.org> on 2022/02/04 03:10:21 UTC
[kudu-CR] 18098: KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Yuqi Du has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18198
Change subject: 18098: KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
18098: KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
Conflicts:
src/kudu/tserver/tablet_service.cc
Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
---
M src/kudu/client/client-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/schema.h
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/all_types-scan-correctness-test.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/compaction.h
M src/kudu/tablet/composite-pushdown-test.cc
M src/kudu/tablet/delta_compaction-test.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_compaction.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/memrowset.cc
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/ops/alter_schema_op.cc
M src/kudu/tablet/ops/alter_schema_op.h
M src/kudu/tablet/ops/op.h
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_metadata.h
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-schema-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tablet/tablet_replica-test-base.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_checksum.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tools/tool_action_perf.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_path_handlers.cc
97 files changed, 667 insertions(+), 570 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18198/1
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 1
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
[kudu-CR] WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18198
to look at the new patch set (#4).
Change subject: WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
Change TabletMeta's variable Schema* to std::shared_ptr<Schema>
to reduce memory used when alter schema.
Because TabletMeta has a old_schemas to reserve the elder schema
when alter schema, maybe they has been used by scanners or
compactions job. But as jira KUDU-3197 said, alter schemas will lead to tserver's
memory too large, just like memory leak, especially column's number is very
large.
wangningito has proposed a simple solution at https://gerrit.cloudera.org/c/16508/.
And I continue his work, and the detail at: https://gerrit.cloudera.org/c/18098/,
But the changes has been thought too much, and code reviewers advise changing from
scoped_refptr<Schema> to std::shared_ptr.
Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
---
M src/kudu/client/client-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/schema.h
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/all_types-scan-correctness-test.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/compaction.h
M src/kudu/tablet/composite-pushdown-test.cc
M src/kudu/tablet/delta_compaction-test.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_compaction.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/memrowset.cc
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/ops/alter_schema_op.cc
M src/kudu/tablet/ops/alter_schema_op.h
M src/kudu/tablet/ops/op.h
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_metadata.h
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-schema-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tablet/tablet_replica-test-base.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_checksum.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tools/tool_action_perf.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_path_handlers.cc
97 files changed, 667 insertions(+), 570 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18198/4
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 4
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18198 )
Change subject: WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
Patch Set 14:
(7 comments)
Fix them. close it and re-commit it.
http://gerrit.cloudera.org:8080/#/c/18198/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18198/5//COMMIT_MSG@18
PS5, Line 18: wangningito has proposed a simple solution at https://gerrit.cloudera.org/c/16508/.
: And I continue his work, and the detail at: https://gerrit.cloudera.org/c/18098/,
: But the changes has been thought too much, and code reviewers advise changing from
: scoped_refptr<Schema> to std::shared_ptr.
> I also left a comment on that ticket mentioning a pretty important detail:
ok, I will study it again.
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/client/client-test.cc@4713
PS5, Line 4713: ColumnSchema col_schema = sch
> In general, the old code just made a copy of the Schema; if you want, you c
ok
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/common/generic_iterators.cc
File src/kudu/common/generic_iterators.cc:
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/common/generic_iterators.cc@518
PS5, Line 518: // Initialized during Init.
: unique_ptr<Schema>
> The problem statement for this ticket only refers to the schemas owned by t
yes. I will restore it.
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/consensus/log.cc
File src/kudu/consensus/log.cc:
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/consensus/log.cc@771
PS5, Line 771: scoped_refptr<Log> new_log(new Log(std::move(optio
> Why not pass the smart pointer directly? Doesn't this mean that we now have
Log no need change, I restore it.
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/master/catalog_manager.cc@2469
PS5, Line 2469: ColumnId* next_col_id) {
: const SchemaPB& current_schema_pb = current
> Again, it seems these changes are unrelated to the schemas referred to in K
Yes. I will remove it
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/master/sys_catalog.cc@342
PS5, Line 342: schema, pa
> Seems like it would be less intrusive to have call-sites use Schema as they
You are right. restore it.
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/tablet/delta_tracker.cc
File src/kudu/tablet/delta_tracker.cc:
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/tablet/delta_tracker.cc@224
PS5, Line 224: opts.projection = projection;
> Without this patch, it seems like this projection is already reference coun
opts.projection is const Schema* before. So I think we should protect the schema.
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 14
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 18 Feb 2022 13:42:03 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18198
to look at the new patch set (#13).
Change subject: WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
Change TabletMeta's variable Schema* to std::shared_ptr<Schema>
to reduce memory used when alter schema.
Because TabletMeta has a old_schemas to reserve the elder schema
when alter schema, maybe they has been used by scanners or
compactions job. But as jira KUDU-3197 said, alter schemas will lead to tserver's
memory too large, just like memory leak, especially column's number is very
large.
wangningito has proposed a simple solution at https://gerrit.cloudera.org/c/16508/.
And I continue his work, and the detail at: https://gerrit.cloudera.org/c/18098/,
But the changes has been thought too much, and code reviewers advise changing from
scoped_refptr<Schema> to std::shared_ptr.
Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
---
M src/kudu/client/client-test.cc
M src/kudu/common/schema.h
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/all_types-scan-correctness-test.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/ops/alter_schema_op.cc
M src/kudu/tablet/ops/alter_schema_op.h
M src/kudu/tablet/ops/op.h
M src/kudu/tablet/rowset_metadata.h
M src/kudu/tablet/tablet-schema-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver_path_handlers.cc
27 files changed, 139 insertions(+), 141 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18198/13
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 13
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18198
to look at the new patch set (#8).
Change subject: WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
Change TabletMeta's variable Schema* to std::shared_ptr<Schema>
to reduce memory used when alter schema.
Because TabletMeta has a old_schemas to reserve the elder schema
when alter schema, maybe they has been used by scanners or
compactions job. But as jira KUDU-3197 said, alter schemas will lead to tserver's
memory too large, just like memory leak, especially column's number is very
large.
wangningito has proposed a simple solution at https://gerrit.cloudera.org/c/16508/.
And I continue his work, and the detail at: https://gerrit.cloudera.org/c/18098/,
But the changes has been thought too much, and code reviewers advise changing from
scoped_refptr<Schema> to std::shared_ptr.
Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
---
M src/kudu/client/client-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/schema.h
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/all_types-scan-correctness-test.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/compaction.h
M src/kudu/tablet/composite-pushdown-test.cc
M src/kudu/tablet/delta_compaction-test.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_compaction.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/memrowset.cc
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/ops/alter_schema_op.cc
M src/kudu/tablet/ops/alter_schema_op.h
M src/kudu/tablet/ops/op.h
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_metadata.h
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-schema-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_checksum.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tools/tool_action_perf.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/tserver_path_handlers.cc
84 files changed, 586 insertions(+), 504 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18198/8
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 8
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18198
to look at the new patch set (#14).
Change subject: WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
Change TabletMeta's variable Schema* to std::shared_ptr<Schema>
to reduce memory used when alter schema.
Because TabletMeta has a old_schemas to reserve the elder schema
when alter schema, maybe they has been used by scanners or
compactions job. But as jira KUDU-3197 said, alter schemas will lead to tserver's
memory too large, just like memory leak, especially column's number is very
large.
wangningito has proposed a simple solution at https://gerrit.cloudera.org/c/16508/.
And I continue his work, and the detail at: https://gerrit.cloudera.org/c/18098/,
But the changes has been thought too much, and code reviewers advise changing from
scoped_refptr<Schema> to std::shared_ptr.
Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
---
M src/kudu/client/client-test.cc
M src/kudu/common/schema.h
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/all_types-scan-correctness-test.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/ops/alter_schema_op.cc
M src/kudu/tablet/ops/alter_schema_op.h
M src/kudu/tablet/ops/op.h
M src/kudu/tablet/rowset_metadata.h
M src/kudu/tablet/tablet-schema-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver_path_handlers.cc
27 files changed, 139 insertions(+), 141 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18198/14
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 14
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18198
to look at the new patch set (#10).
Change subject: WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
Change TabletMeta's variable Schema* to std::shared_ptr<Schema>
to reduce memory used when alter schema.
Because TabletMeta has a old_schemas to reserve the elder schema
when alter schema, maybe they has been used by scanners or
compactions job. But as jira KUDU-3197 said, alter schemas will lead to tserver's
memory too large, just like memory leak, especially column's number is very
large.
wangningito has proposed a simple solution at https://gerrit.cloudera.org/c/16508/.
And I continue his work, and the detail at: https://gerrit.cloudera.org/c/18098/,
But the changes has been thought too much, and code reviewers advise changing from
scoped_refptr<Schema> to std::shared_ptr.
Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
---
M src/kudu/client/client-test.cc
M src/kudu/common/schema.h
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/all_types-scan-correctness-test.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/compaction.h
M src/kudu/tablet/composite-pushdown-test.cc
M src/kudu/tablet/delta_compaction-test.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_compaction.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/memrowset.cc
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/ops/alter_schema_op.cc
M src/kudu/tablet/ops/alter_schema_op.h
M src/kudu/tablet/ops/op.h
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_metadata.h
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-schema-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver_path_handlers.cc
66 files changed, 489 insertions(+), 434 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18198/10
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 10
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18198 )
Change subject: WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
Patch Set 5:
(9 comments)
I started going through the non-test files here. My overarching thoughts are that this patch could be significantly less invasive if we just focus on the problem at hand: the Schemas in TabletMetadata::old_schemas_, rather than Schemas as a whole.
http://gerrit.cloudera.org:8080/#/c/18198/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18198/5//COMMIT_MSG@18
PS5, Line 18: wangningito has proposed a simple solution at https://gerrit.cloudera.org/c/16508/.
: And I continue his work, and the detail at: https://gerrit.cloudera.org/c/18098/,
: But the changes has been thought too much, and code reviewers advise changing from
: scoped_refptr<Schema> to std::shared_ptr.
I also left a comment on that ticket mentioning a pretty important detail:
https://issues.apache.org/jira/browse/KUDU-3197?focusedCommentId=17203521&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17203521
The schemas that are affected by this bug are only the ones used and passed around by TabletMetadata::schema(). If that is the case, would it make sense to just start there, updating the callers of TabletMetadata::schema() to use shared_ptr<>, and leaving the rest of our usages of Schema untouched?
That is, the general usage of Schemas today is already safe, and it's only the schemas in TabletMetadata::old_schemas_ that are ever affected by this bug. I appreciate the attention across the codebase to fixing this, but something targeted seems more appropriate and less likely to introduce a performance regression (right now, we may be doing more heap allocation than before).
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/client/client-test.cc@4713
PS5, Line 4713: Schema& schema = *schema_ptr;
In general, the old code just made a copy of the Schema; if you want, you could do the same here and elsewhere, for example:
Schema schema = *tablet_replica->tablet()->metadata()->schema();
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/common/generic_iterators.cc
File src/kudu/common/generic_iterators.cc:
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/common/generic_iterators.cc@518
PS5, Line 518: // Initialized during Init.
: SchemaPtr schema_;
The problem statement for this ticket only refers to the schemas owned by the TabletMetadata, if I understand correctly. Since this schema is wholly owned by this iterator, we should be fine leaving it as it was. Same below.
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/consensus/log.cc
File src/kudu/consensus/log.cc:
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/consensus/log.cc@771
PS5, Line 771: SchemaPtr schema_clone(new Schema(*schema.get()));
Why not pass the smart pointer directly? Doesn't this mean that we now have two heap-allocated schemas?
If we're not going to share references to the same schema object, we may as well leave this to be copied on the stack as it was before (i.e. using Schema instead of SchemaPtr).
Same elsewhere in this file.
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/master/catalog_manager.cc@2469
PS5, Line 2469: SchemaPtr cur_schema_ptr = std::make_shared<Schema>();
: Schema& cur_schema = *cur_schema_ptr.get();
Again, it seems these changes are unrelated to the schemas referred to in KUDU-3197. We should leave them as is.
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/master/sys_catalog.cc@342
PS5, Line 342: schema_ptr
Seems like it would be less intrusive to have call-sites use Schema as they did before, but have CreateNew() convert it to a std::shared_ptr<Schema>, as done here. What do you think?
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/master/sys_catalog.cc@786
PS5, Line 786: schema_ptr
Seems unnecessary to use this, given the iterator already makes a copy and thus has full control over the life cycle of its schema. Here we're doing the same, but heap-allocating the schema.
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/tablet/cfile_set.h
File src/kudu/tablet/cfile_set.h:
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/tablet/cfile_set.h@87
PS5, Line 87: SchemaPtr
Following the trend of iterators owning their schemas, I think we can leave this as a raw pointer, since the Schema it points to isn't the one owned by the TabletMetadata
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/tablet/delta_tracker.cc
File src/kudu/tablet/delta_tracker.cc:
http://gerrit.cloudera.org:8080/#/c/18198/5/src/kudu/tablet/delta_tracker.cc@224
PS5, Line 224: opts.projection = projection;
Without this patch, it seems like this projection is already reference counted (via unique_ptr) and fully owned by the iterator. That seems to be safe as is.
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 5
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 05 Feb 2022 00:39:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18198
to look at the new patch set (#7).
Change subject: WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
Change TabletMeta's variable Schema* to std::shared_ptr<Schema>
to reduce memory used when alter schema.
Because TabletMeta has a old_schemas to reserve the elder schema
when alter schema, maybe they has been used by scanners or
compactions job. But as jira KUDU-3197 said, alter schemas will lead to tserver's
memory too large, just like memory leak, especially column's number is very
large.
wangningito has proposed a simple solution at https://gerrit.cloudera.org/c/16508/.
And I continue his work, and the detail at: https://gerrit.cloudera.org/c/18098/,
But the changes has been thought too much, and code reviewers advise changing from
scoped_refptr<Schema> to std::shared_ptr.
Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
---
M src/kudu/client/client-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/schema.h
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/all_types-scan-correctness-test.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/compaction.h
M src/kudu/tablet/composite-pushdown-test.cc
M src/kudu/tablet/delta_compaction-test.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_compaction.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/memrowset.cc
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/ops/alter_schema_op.cc
M src/kudu/tablet/ops/alter_schema_op.h
M src/kudu/tablet/ops/op.h
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_metadata.h
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-schema-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tablet/tablet_replica-test-base.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_checksum.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tools/tool_action_perf.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/tserver_path_handlers.cc
94 files changed, 621 insertions(+), 533 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18198/7
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 7
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18198
to look at the new patch set (#2).
Change subject: WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
Change TabletMeta's variable Schema* to std::shared_ptr<Schema>
to reduce memory used when alter schema.
Because TabletMeta has a old_schemas to reserve the elder schema
when alter schema, maybe they has been used by scanners or
compactions job. But as jira KUDU-3197 said, alter schemas will lead to tserver's
memory too large, just like memory leak, especially column's number is very
large.
wangningito has proposed a simple solution at https://gerrit.cloudera.org/c/16508/.
And I continue his work, and the detail at: https://gerrit.cloudera.org/c/18098/,
But the changes has been thought too much, and code reviewers advise changing from
scoped_refptr<Schema> to std::shared_ptr.
Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
---
M src/kudu/client/client-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/schema.h
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/all_types-scan-correctness-test.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/compaction.h
M src/kudu/tablet/composite-pushdown-test.cc
M src/kudu/tablet/delta_compaction-test.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_compaction.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/memrowset.cc
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/ops/alter_schema_op.cc
M src/kudu/tablet/ops/alter_schema_op.h
M src/kudu/tablet/ops/op.h
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_metadata.h
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-schema-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tablet/tablet_replica-test-base.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_checksum.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tools/tool_action_perf.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_path_handlers.cc
97 files changed, 667 insertions(+), 570 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18198/2
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 2
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18198
to look at the new patch set (#3).
Change subject: WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
Change TabletMeta's variable Schema* to std::shared_ptr<Schema>
to reduce memory used when alter schema.
Because TabletMeta has a old_schemas to reserve the elder schema
when alter schema, maybe they has been used by scanners or
compactions job. But as jira KUDU-3197 said, alter schemas will lead to tserver's
memory too large, just like memory leak, especially column's number is very
large.
wangningito has proposed a simple solution at https://gerrit.cloudera.org/c/16508/.
And I continue his work, and the detail at: https://gerrit.cloudera.org/c/18098/,
But the changes has been thought too much, and code reviewers advise changing from
scoped_refptr<Schema> to std::shared_ptr.
Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
---
M src/kudu/client/client-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/schema.h
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/all_types-scan-correctness-test.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/compaction.h
M src/kudu/tablet/composite-pushdown-test.cc
M src/kudu/tablet/delta_compaction-test.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_compaction.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/memrowset.cc
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/ops/alter_schema_op.cc
M src/kudu/tablet/ops/alter_schema_op.h
M src/kudu/tablet/ops/op.h
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_metadata.h
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-schema-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tablet/tablet_replica-test-base.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_checksum.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tools/tool_action_perf.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_path_handlers.cc
97 files changed, 667 insertions(+), 570 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18198/3
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 3
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18198
to look at the new patch set (#5).
Change subject: WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
Change TabletMeta's variable Schema* to std::shared_ptr<Schema>
to reduce memory used when alter schema.
Because TabletMeta has a old_schemas to reserve the elder schema
when alter schema, maybe they has been used by scanners or
compactions job. But as jira KUDU-3197 said, alter schemas will lead to tserver's
memory too large, just like memory leak, especially column's number is very
large.
wangningito has proposed a simple solution at https://gerrit.cloudera.org/c/16508/.
And I continue his work, and the detail at: https://gerrit.cloudera.org/c/18098/,
But the changes has been thought too much, and code reviewers advise changing from
scoped_refptr<Schema> to std::shared_ptr.
Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
---
M src/kudu/client/client-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/schema.h
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/all_types-scan-correctness-test.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/compaction.h
M src/kudu/tablet/composite-pushdown-test.cc
M src/kudu/tablet/delta_compaction-test.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_compaction.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/memrowset.cc
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/ops/alter_schema_op.cc
M src/kudu/tablet/ops/alter_schema_op.h
M src/kudu/tablet/ops/op.h
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_metadata.h
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-schema-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tablet/tablet_replica-test-base.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_checksum.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tools/tool_action_perf.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_path_handlers.cc
97 files changed, 667 insertions(+), 570 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18198/5
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 5
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18198
to look at the new patch set (#6).
Change subject: WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
Change TabletMeta's variable Schema* to std::shared_ptr<Schema>
to reduce memory used when alter schema.
Because TabletMeta has a old_schemas to reserve the elder schema
when alter schema, maybe they has been used by scanners or
compactions job. But as jira KUDU-3197 said, alter schemas will lead to tserver's
memory too large, just like memory leak, especially column's number is very
large.
wangningito has proposed a simple solution at https://gerrit.cloudera.org/c/16508/.
And I continue his work, and the detail at: https://gerrit.cloudera.org/c/18098/,
But the changes has been thought too much, and code reviewers advise changing from
scoped_refptr<Schema> to std::shared_ptr.
Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
---
M src/kudu/client/client-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/schema.h
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/all_types-scan-correctness-test.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/compaction.h
M src/kudu/tablet/composite-pushdown-test.cc
M src/kudu/tablet/delta_compaction-test.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_compaction.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/memrowset.cc
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/ops/alter_schema_op.cc
M src/kudu/tablet/ops/alter_schema_op.h
M src/kudu/tablet/ops/op.h
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_metadata.h
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-schema-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tablet/tablet_replica-test-base.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_checksum.cc
M src/kudu/tools/ksck_checksum.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tools/tool_action_perf.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_path_handlers.cc
97 files changed, 657 insertions(+), 566 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18198/6
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 6
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18198
to look at the new patch set (#11).
Change subject: WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
Change TabletMeta's variable Schema* to std::shared_ptr<Schema>
to reduce memory used when alter schema.
Because TabletMeta has a old_schemas to reserve the elder schema
when alter schema, maybe they has been used by scanners or
compactions job. But as jira KUDU-3197 said, alter schemas will lead to tserver's
memory too large, just like memory leak, especially column's number is very
large.
wangningito has proposed a simple solution at https://gerrit.cloudera.org/c/16508/.
And I continue his work, and the detail at: https://gerrit.cloudera.org/c/18098/,
But the changes has been thought too much, and code reviewers advise changing from
scoped_refptr<Schema> to std::shared_ptr.
Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
---
M src/kudu/client/client-test.cc
M src/kudu/common/schema.h
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/all_types-scan-correctness-test.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/compaction.h
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/memrowset.cc
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/ops/alter_schema_op.cc
M src/kudu/tablet/ops/alter_schema_op.h
M src/kudu/tablet/ops/op.h
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_metadata.h
M src/kudu/tablet/tablet-schema-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver_path_handlers.cc
38 files changed, 245 insertions(+), 236 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18198/11
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 11
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18198
to look at the new patch set (#9).
Change subject: WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
Change TabletMeta's variable Schema* to std::shared_ptr<Schema>
to reduce memory used when alter schema.
Because TabletMeta has a old_schemas to reserve the elder schema
when alter schema, maybe they has been used by scanners or
compactions job. But as jira KUDU-3197 said, alter schemas will lead to tserver's
memory too large, just like memory leak, especially column's number is very
large.
wangningito has proposed a simple solution at https://gerrit.cloudera.org/c/16508/.
And I continue his work, and the detail at: https://gerrit.cloudera.org/c/18098/,
But the changes has been thought too much, and code reviewers advise changing from
scoped_refptr<Schema> to std::shared_ptr.
Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
---
M src/kudu/client/client-test.cc
M src/kudu/common/schema.h
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/all_types-scan-correctness-test.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/compaction.h
M src/kudu/tablet/composite-pushdown-test.cc
M src/kudu/tablet/delta_compaction-test.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_compaction.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/delta_tracker.cc
M src/kudu/tablet/delta_tracker.h
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltafile.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/diskrowset.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/memrowset.cc
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mock-rowsets.h
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/ops/alter_schema_op.cc
M src/kudu/tablet/ops/alter_schema_op.h
M src/kudu/tablet/ops/op.h
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/rowset_metadata.h
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-schema-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/tserver_path_handlers.cc
70 files changed, 520 insertions(+), 453 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18198/9
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 9
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18198
to look at the new patch set (#12).
Change subject: WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
Change TabletMeta's variable Schema* to std::shared_ptr<Schema>
to reduce memory used when alter schema.
Because TabletMeta has a old_schemas to reserve the elder schema
when alter schema, maybe they has been used by scanners or
compactions job. But as jira KUDU-3197 said, alter schemas will lead to tserver's
memory too large, just like memory leak, especially column's number is very
large.
wangningito has proposed a simple solution at https://gerrit.cloudera.org/c/16508/.
And I continue his work, and the detail at: https://gerrit.cloudera.org/c/18098/,
But the changes has been thought too much, and code reviewers advise changing from
scoped_refptr<Schema> to std::shared_ptr.
Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
---
M src/kudu/client/client-test.cc
M src/kudu/common/schema.h
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/all_types-scan-correctness-test.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/ops/alter_schema_op.cc
M src/kudu/tablet/ops/alter_schema_op.h
M src/kudu/tablet/ops/op.h
M src/kudu/tablet/ops/write_op.h
M src/kudu/tablet/rowset_metadata.h
M src/kudu/tablet/tablet-schema-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver_path_handlers.cc
28 files changed, 134 insertions(+), 138 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18198/12
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 12
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared ptr
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has abandoned this change. ( http://gerrit.cloudera.org:8080/18198 )
Change subject: WIP KUDU-3197 [tserver] optimal Schema's memory used, using std::shared_ptr
......................................................................
Abandoned
It's replaced by https://gerrit.cloudera.org/c/18255/
--
To view, visit http://gerrit.cloudera.org:8080/18198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I586e3a67791de4006c3479b39f3222cbf93fac28
Gerrit-Change-Number: 18198
Gerrit-PatchSet: 14
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>