You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Anonymous Coward (Code Review)" <ge...@cloudera.org> on 2021/12/14 03:27:41 UTC

[kudu-CR] KUDU-3197 fix TabletMeta's variable Schema* to scoped refptr, to reduce memory used

shenxingwuying@gmail.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18098


Change subject: KUDU-3197 fix TabletMeta's variable Schema* to scoped_refptr<Schema>, to reduce memory used
......................................................................

KUDU-3197 fix TabletMeta's variable Schema* to scoped_refptr<Schema>, to reduce memory used

Need more details explains:

Conflicts:
	src/kudu/master/catalog_manager.cc
	src/kudu/master/catalog_manager.h

Conflicts:
	src/kudu/cfile/cfile-test.cc
	src/kudu/client/client-internal.cc
	src/kudu/client/client.cc
	src/kudu/client/scan_configuration.cc
	src/kudu/client/scan_token-internal.cc
	src/kudu/client/schema.cc
	src/kudu/common/encoded_key-test.cc
	src/kudu/common/generic_iterators.cc
	src/kudu/common/partition-test.cc
	src/kudu/common/partition.cc
	src/kudu/common/partition.h
	src/kudu/common/partition_pruner-test.cc
	src/kudu/common/row_operations-test.cc
	src/kudu/common/scan_spec-test.cc
	src/kudu/common/wire_protocol-test.cc
	src/kudu/hms/hms_catalog-test.cc
	src/kudu/integration-tests/table_locations-itest.cc
	src/kudu/master/catalog_manager.cc
	src/kudu/master/catalog_manager.h
	src/kudu/master/master-test.cc
	src/kudu/master/sentry_authz_provider-test.cc
	src/kudu/master/sys_catalog.cc
	src/kudu/tablet/all_types-scan-correctness-test.cc
	src/kudu/tablet/cfile_set-test.cc
	src/kudu/tablet/compaction-test.cc
	src/kudu/tablet/compaction.cc
	src/kudu/tablet/delta_compaction.cc
	src/kudu/tablet/deltafile-test.cc
	src/kudu/tablet/deltamemstore-test.cc
	src/kudu/tablet/diskrowset-test-base.h
	src/kudu/tablet/diskrowset.cc
	src/kudu/tablet/memrowset-test.cc
	src/kudu/tablet/memrowset.cc
	src/kudu/tablet/memrowset.h
	src/kudu/tablet/mock-rowsets.h
	src/kudu/tablet/mt-rowset_delta_compaction-test.cc
	src/kudu/tablet/mt-tablet-test.cc
	src/kudu/tablet/ops/alter_schema_op.cc
	src/kudu/tablet/tablet-decoder-eval-test.cc
	src/kudu/tablet/tablet-pushdown-test.cc
	src/kudu/tablet/tablet-test-base.h
	src/kudu/tablet/tablet-test-util.h
	src/kudu/tablet/tablet-test.cc
	src/kudu/tablet/tablet.cc
	src/kudu/tablet/tablet_bootstrap.cc
	src/kudu/tablet/tablet_metadata.cc
	src/kudu/tablet/tablet_random_access-test.cc
	src/kudu/tablet/tablet_replica-test.cc
	src/kudu/tools/ksck-test.cc
	src/kudu/tools/ksck_remote.cc
	src/kudu/tools/kudu-tool-test.cc
	src/kudu/tools/tool_action_fs.cc
	src/kudu/tools/tool_action_hms.cc
	src/kudu/tools/tool_action_master.cc
	src/kudu/tools/tool_action_perf.cc
	src/kudu/tserver/scanners.cc
	src/kudu/tserver/scanners.h
	src/kudu/tserver/tablet_copy_client.cc
	src/kudu/tserver/tablet_server-test-base.cc
	src/kudu/tserver/tablet_server-test.cc
	src/kudu/tserver/tablet_server_authorization-test.cc
	src/kudu/tserver/tablet_service.cc

Conflicts:
	src/kudu/cfile/cfile-test.cc
	src/kudu/client/client-test.cc
	src/kudu/client/client.cc
	src/kudu/client/meta_cache.cc
	src/kudu/client/scan_configuration.cc
	src/kudu/client/scan_configuration.h
	src/kudu/common/partition-test.cc
	src/kudu/common/partition.cc
	src/kudu/common/partition.h
	src/kudu/common/partition_pruner-test.cc
	src/kudu/common/row_operations-test.cc
	src/kudu/common/scan_spec-test.cc
	src/kudu/hms/hms_catalog-test.cc
	src/kudu/integration-tests/fuzz-itest.cc
	src/kudu/integration-tests/master_hms-itest.cc
	src/kudu/integration-tests/security-itest.cc
	src/kudu/integration-tests/table_locations-itest.cc
	src/kudu/integration-tests/tablet_replacement-itest.cc
	src/kudu/integration-tests/txn_participant-itest.cc
	src/kudu/master/catalog_manager.cc
	src/kudu/master/master-test.cc
	src/kudu/master/master.cc
	src/kudu/master/master.h
	src/kudu/tablet/cfile_set.cc
	src/kudu/tablet/delta_store.h
	src/kudu/tablet/ops/alter_schema_op.cc
	src/kudu/tablet/ops/alter_schema_op.h
	src/kudu/tablet/tablet-test-base.h
	src/kudu/tablet/tablet.cc
	src/kudu/tablet/tablet_metadata.cc
	src/kudu/tablet/tablet_metadata.h
	src/kudu/tools/ksck_remote.cc
	src/kudu/tools/kudu-tool-test.cc
	src/kudu/tools/master_rebuilder.cc
	src/kudu/tools/table_scanner.cc
	src/kudu/tools/tool_action_hms.cc
	src/kudu/tools/tool_action_perf.cc
	src/kudu/tools/tool_action_table.cc
	src/kudu/transactions/txn_system_client.cc
	src/kudu/tserver/tablet_server_authorization-test.cc
	src/kudu/tserver/ts_tablet_manager-test.cc

Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/client/batcher.cc
M src/kudu/client/client-internal.cc
M src/kudu/client/client-test.cc
M src/kudu/client/client-unittest.cc
M src/kudu/client/client.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/scan_configuration.cc
M src/kudu/client/scan_configuration.h
M src/kudu/client/scan_token-internal.cc
M src/kudu/client/scanner-internal.cc
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/client/table-internal.h
M src/kudu/client/write_op.cc
M src/kudu/codegen/codegen-test.cc
M src/kudu/codegen/compilation_manager.cc
M src/kudu/codegen/row_projector.cc
M src/kudu/codegen/row_projector.h
M src/kudu/common/encoded_key-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/iterator.h
M src/kudu/common/key_util-test.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/row_operations-test.cc
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/schema-test.cc
M src/kudu/common/schema.cc
M src/kudu/common/schema.h
M src/kudu/common/wire_protocol-test-util.h
M src/kudu/common/wire_protocol-test.cc
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-test.cc
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/hms/hms_catalog-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/auth_token_expire-itest.cc
M src/kudu/integration-tests/authz_token-itest.cc
M src/kudu/integration-tests/catalog_manager_tsk-itest.cc
M src/kudu/integration-tests/client-negotiation-failover-itest.cc
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/create-table-itest.cc
M src/kudu/integration-tests/delete_table-itest.cc
M src/kudu/integration-tests/disk_failure-itest.cc
M src/kudu/integration-tests/exactly_once_writes-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/raft_consensus-itest-base.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/security-faults-itest.cc
M src/kudu/integration-tests/security-itest.cc
M src/kudu/integration-tests/security-unknown-tsk-itest.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/tablet_replacement-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/integration-tests/ts_itest-base.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/integration-tests/txn_commit-itest.cc
M src/kudu/integration-tests/txn_participant-itest.cc
M src/kudu/integration-tests/txn_write_ops-itest.cc
M src/kudu/integration-tests/write_limit-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
M src/kudu/master/master.h
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/master/ts_state-test.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_applier.cc
M src/kudu/tablet/delta_applier.h
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/key_value_test_schema.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.cc
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_history_gc-test.cc
M src/kudu/tablet/tablet_metadata-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-base.cc
M src/kudu/tablet/tablet_replica-test-base.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_throttle-test.cc
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tools/create-table-tool-test.cc
M src/kudu/tools/ksck-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/ksck_remote.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_hms.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/tools/tool_action_table.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/transactions/txn_status_tablet.h
M src/kudu/transactions/txn_system_client.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
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-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_server_authorization-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
M src/kudu/util/subprocess.cc
185 files changed, 2,560 insertions(+), 2,110 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18098/1
-- 
To view, visit http://gerrit.cloudera.org:8080/18098
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
Gerrit-Change-Number: 18098
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>

[kudu-CR] KUDU-3197 [tserver] optimal Schema's memory used

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18098

to look at the new patch set (#2).

Change subject: KUDU-3197 [tserver] optimal Schema's memory used
......................................................................

KUDU-3197 [tserver] optimal Schema's memory used

change TabletMeta's variable Schema* to scoped_refptr<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 thought looks like simple, but implements is a
little complex, for Schema has changed to scoped_refptr<Schema>, leading to
some writes not correct before, I must have to repaire a lot of unit tests
and some others codes.

Conflicts:
	src/kudu/master/catalog_manager.cc
	src/kudu/master/catalog_manager.h

Conflicts:
	src/kudu/cfile/cfile-test.cc
	src/kudu/client/client-internal.cc
	src/kudu/client/client.cc
	src/kudu/client/scan_configuration.cc
	src/kudu/client/scan_token-internal.cc
	src/kudu/client/schema.cc
	src/kudu/common/encoded_key-test.cc
	src/kudu/common/generic_iterators.cc
	src/kudu/common/partition-test.cc
	src/kudu/common/partition.cc
	src/kudu/common/partition.h
	src/kudu/common/partition_pruner-test.cc
	src/kudu/common/row_operations-test.cc
	src/kudu/common/scan_spec-test.cc
	src/kudu/common/wire_protocol-test.cc
	src/kudu/hms/hms_catalog-test.cc
	src/kudu/integration-tests/table_locations-itest.cc
	src/kudu/master/catalog_manager.cc
	src/kudu/master/catalog_manager.h
	src/kudu/master/master-test.cc
	src/kudu/master/sentry_authz_provider-test.cc
	src/kudu/master/sys_catalog.cc
	src/kudu/tablet/all_types-scan-correctness-test.cc
	src/kudu/tablet/cfile_set-test.cc
	src/kudu/tablet/compaction-test.cc
	src/kudu/tablet/compaction.cc
	src/kudu/tablet/delta_compaction.cc
	src/kudu/tablet/deltafile-test.cc
	src/kudu/tablet/deltamemstore-test.cc
	src/kudu/tablet/diskrowset-test-base.h
	src/kudu/tablet/diskrowset.cc
	src/kudu/tablet/memrowset-test.cc
	src/kudu/tablet/memrowset.cc
	src/kudu/tablet/memrowset.h
	src/kudu/tablet/mock-rowsets.h
	src/kudu/tablet/mt-rowset_delta_compaction-test.cc
	src/kudu/tablet/mt-tablet-test.cc
	src/kudu/tablet/ops/alter_schema_op.cc
	src/kudu/tablet/tablet-decoder-eval-test.cc
	src/kudu/tablet/tablet-pushdown-test.cc
	src/kudu/tablet/tablet-test-base.h
	src/kudu/tablet/tablet-test-util.h
	src/kudu/tablet/tablet-test.cc
	src/kudu/tablet/tablet.cc
	src/kudu/tablet/tablet_bootstrap.cc
	src/kudu/tablet/tablet_metadata.cc
	src/kudu/tablet/tablet_random_access-test.cc
	src/kudu/tablet/tablet_replica-test.cc
	src/kudu/tools/ksck-test.cc
	src/kudu/tools/ksck_remote.cc
	src/kudu/tools/kudu-tool-test.cc
	src/kudu/tools/tool_action_fs.cc
	src/kudu/tools/tool_action_hms.cc
	src/kudu/tools/tool_action_master.cc
	src/kudu/tools/tool_action_perf.cc
	src/kudu/tserver/scanners.cc
	src/kudu/tserver/scanners.h
	src/kudu/tserver/tablet_copy_client.cc
	src/kudu/tserver/tablet_server-test-base.cc
	src/kudu/tserver/tablet_server-test.cc
	src/kudu/tserver/tablet_server_authorization-test.cc
	src/kudu/tserver/tablet_service.cc

Conflicts:
	src/kudu/cfile/cfile-test.cc
	src/kudu/client/client-test.cc
	src/kudu/client/client.cc
	src/kudu/client/meta_cache.cc
	src/kudu/client/scan_configuration.cc
	src/kudu/client/scan_configuration.h
	src/kudu/common/partition-test.cc
	src/kudu/common/partition.cc
	src/kudu/common/partition.h
	src/kudu/common/partition_pruner-test.cc
	src/kudu/common/row_operations-test.cc
	src/kudu/common/scan_spec-test.cc
	src/kudu/hms/hms_catalog-test.cc
	src/kudu/integration-tests/fuzz-itest.cc
	src/kudu/integration-tests/master_hms-itest.cc
	src/kudu/integration-tests/security-itest.cc
	src/kudu/integration-tests/table_locations-itest.cc
	src/kudu/integration-tests/tablet_replacement-itest.cc
	src/kudu/integration-tests/txn_participant-itest.cc
	src/kudu/master/catalog_manager.cc
	src/kudu/master/master-test.cc
	src/kudu/master/master.cc
	src/kudu/master/master.h
	src/kudu/tablet/cfile_set.cc
	src/kudu/tablet/delta_store.h
	src/kudu/tablet/ops/alter_schema_op.cc
	src/kudu/tablet/ops/alter_schema_op.h
	src/kudu/tablet/tablet-test-base.h
	src/kudu/tablet/tablet.cc
	src/kudu/tablet/tablet_metadata.cc
	src/kudu/tablet/tablet_metadata.h
	src/kudu/tools/ksck_remote.cc
	src/kudu/tools/kudu-tool-test.cc
	src/kudu/tools/master_rebuilder.cc
	src/kudu/tools/table_scanner.cc
	src/kudu/tools/tool_action_hms.cc
	src/kudu/tools/tool_action_perf.cc
	src/kudu/tools/tool_action_table.cc
	src/kudu/transactions/txn_system_client.cc
	src/kudu/tserver/tablet_server_authorization-test.cc
	src/kudu/tserver/ts_tablet_manager-test.cc

Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/client/batcher.cc
M src/kudu/client/client-internal.cc
M src/kudu/client/client-test.cc
M src/kudu/client/client-unittest.cc
M src/kudu/client/client.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/scan_configuration.cc
M src/kudu/client/scan_configuration.h
M src/kudu/client/scan_token-internal.cc
M src/kudu/client/scanner-internal.cc
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/client/table-internal.h
M src/kudu/client/write_op.cc
M src/kudu/codegen/codegen-test.cc
M src/kudu/codegen/compilation_manager.cc
M src/kudu/codegen/row_projector.cc
M src/kudu/codegen/row_projector.h
M src/kudu/common/encoded_key-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/iterator.h
M src/kudu/common/key_util-test.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/row_operations-test.cc
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/schema-test.cc
M src/kudu/common/schema.cc
M src/kudu/common/schema.h
M src/kudu/common/wire_protocol-test-util.h
M src/kudu/common/wire_protocol-test.cc
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-test.cc
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/hms/hms_catalog-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/auth_token_expire-itest.cc
M src/kudu/integration-tests/authz_token-itest.cc
M src/kudu/integration-tests/catalog_manager_tsk-itest.cc
M src/kudu/integration-tests/client-negotiation-failover-itest.cc
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/create-table-itest.cc
M src/kudu/integration-tests/delete_table-itest.cc
M src/kudu/integration-tests/disk_failure-itest.cc
M src/kudu/integration-tests/exactly_once_writes-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/raft_consensus-itest-base.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/security-faults-itest.cc
M src/kudu/integration-tests/security-itest.cc
M src/kudu/integration-tests/security-unknown-tsk-itest.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/tablet_replacement-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/integration-tests/ts_itest-base.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/integration-tests/txn_commit-itest.cc
M src/kudu/integration-tests/txn_participant-itest.cc
M src/kudu/integration-tests/txn_write_ops-itest.cc
M src/kudu/integration-tests/write_limit-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
M src/kudu/master/master.h
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/master/ts_state-test.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_applier.cc
M src/kudu/tablet/delta_applier.h
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/key_value_test_schema.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.cc
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_history_gc-test.cc
M src/kudu/tablet/tablet_metadata-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-base.cc
M src/kudu/tablet/tablet_replica-test-base.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_throttle-test.cc
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tools/create-table-tool-test.cc
M src/kudu/tools/ksck-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/ksck_remote.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_hms.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/tools/tool_action_table.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/transactions/txn_status_tablet.h
M src/kudu/transactions/txn_system_client.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
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-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_server_authorization-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
M src/kudu/util/subprocess.cc
185 files changed, 2,476 insertions(+), 2,111 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18098/2
-- 
To view, visit http://gerrit.cloudera.org:8080/18098
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
Gerrit-Change-Number: 18098
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-3197 [tserver] optimal Schema's memory used

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18098

to look at the new patch set (#3).

Change subject: KUDU-3197 [tserver] optimal Schema's memory used
......................................................................

KUDU-3197 [tserver] optimal Schema's memory used

change TabletMeta's variable Schema* to scoped_refptr<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 thought looks like simple, but implements is a
little complex, for Schema has changed to scoped_refptr<Schema>, leading to
some writes not correct before, I must have to repaire a lot of unit tests
and some others codes.

Conflicts:
	src/kudu/cfile/cfile-test.cc
	src/kudu/client/client-internal.cc
	src/kudu/client/client.cc
	src/kudu/client/meta_cache.cc
	src/kudu/client/scan_configuration.h
	src/kudu/client/scan_configuration.cc
	src/kudu/client/scan_token-internal.cc
	src/kudu/client/schema.cc
	src/kudu/common/encoded_key-test.cc
	src/kudu/common/generic_iterators.cc
	src/kudu/common/partition-test.cc
	src/kudu/common/partition.cc
	src/kudu/common/partition.h
	src/kudu/common/partition_pruner-test.cc
	src/kudu/common/row_operations-test.cc
	src/kudu/common/scan_spec-test.cc
	src/kudu/common/wire_protocol-test.cc
	src/kudu/hms/hms_catalog-test.cc
	src/kudu/integration-tests/fuzz-itest.cc
	src/kudu/integration-tests/master_hms-itest.cc
	src/kudu/integration-tests/security-itest.cc
	src/kudu/integration-tests/table_locations-itest.cc
	src/kudu/integration-tests/tablet_replacement-itest.cc
	src/kudu/integration-tests/txn_participant-itest.cc
	src/kudu/master/catalog_manager.cc
	src/kudu/master/catalog_manager.h
	src/kudu/master/master-test.cc
	src/kudu/master/master.cc
	src/kudu/master/master.h
	src/kudu/master/sentry_authz_provider-test.cc
	src/kudu/master/sys_catalog.cc
	src/kudu/tablet/all_types-scan-correctness-test.cc
	src/kudu/tablet/cfile_set-test.cc
	src/kudu/tablet/cfile_set.cc
	src/kudu/tablet/compaction-test.cc
	src/kudu/tablet/compaction.cc
	src/kudu/tablet/delta_compaction.cc
	src/kudu/tablet/deltafile-test.cc
	src/kudu/tablet/delta_store.h
	src/kudu/tablet/deltamemstore-test.cc
	src/kudu/tablet/diskrowset-test-base.h
	src/kudu/tablet/diskrowset.cc
	src/kudu/tablet/memrowset-test.cc
	src/kudu/tablet/memrowset.cc
	src/kudu/tablet/memrowset.h
	src/kudu/tablet/mock-rowsets.h
	src/kudu/tablet/mt-rowset_delta_compaction-test.cc
	src/kudu/tablet/mt-tablet-test.cc
	src/kudu/tablet/ops/alter_schema_op.h
	src/kudu/tablet/ops/alter_schema_op.cc
	src/kudu/tablet/tablet-decoder-eval-test.cc
	src/kudu/tablet/tablet-pushdown-test.cc
	src/kudu/tablet/tablet-test-base.h
	src/kudu/tablet/tablet-test-util.h
	src/kudu/tablet/tablet-test.cc
	src/kudu/tablet/tablet.cc
	src/kudu/tablet/tablet_bootstrap.cc
	src/kudu/tablet/tablet_metadata.cc
	src/kudu/tablet/tablet_random_access-test.cc
	src/kudu/tablet/tablet_replica-test.cc
	src/kudu/tablet/tablet-test-base.h
	src/kudu/tablet/tablet_metadata.cc
	src/kudu/tablet/tablet_metadata.h
	src/kudu/tools/master_rebuilder.cc
	src/kudu/tools/ksck-test.cc
	src/kudu/tools/ksck_remote.cc
	src/kudu/tools/kudu-tool-test.cc
	src/kudu/tools/table_scanner.cc
	src/kudu/tools/tool_action_fs.cc
	src/kudu/tools/tool_action_hms.cc
	src/kudu/tools/tool_action_master.cc
	src/kudu/tools/tool_action_perf.cc
	src/kudu/tools/tool_action_table.cc
	src/kudu/transactions/txn_system_client.cc
	src/kudu/tserver/scanners.cc
	src/kudu/tserver/scanners.h
	src/kudu/tserver/tablet_copy_client.cc
	src/kudu/tserver/tablet_server-test-base.cc
	src/kudu/tserver/tablet_server-test.cc
	src/kudu/tserver/tablet_server_authorization-test.cc
	src/kudu/tserver/tablet_service.cc
	src/kudu/tserver/tablet_server_authorization-test.cc
	src/kudu/tserver/ts_tablet_manager-test.cc

Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/client/batcher.cc
M src/kudu/client/client-internal.cc
M src/kudu/client/client-test.cc
M src/kudu/client/client-unittest.cc
M src/kudu/client/client.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/scan_configuration.cc
M src/kudu/client/scan_configuration.h
M src/kudu/client/scan_token-internal.cc
M src/kudu/client/scanner-internal.cc
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/client/table-internal.h
M src/kudu/client/write_op.cc
M src/kudu/codegen/codegen-test.cc
M src/kudu/codegen/compilation_manager.cc
M src/kudu/codegen/row_projector.cc
M src/kudu/codegen/row_projector.h
M src/kudu/common/encoded_key-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/iterator.h
M src/kudu/common/key_util-test.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/row_operations-test.cc
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/schema-test.cc
M src/kudu/common/schema.cc
M src/kudu/common/schema.h
M src/kudu/common/wire_protocol-test-util.h
M src/kudu/common/wire_protocol-test.cc
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-test.cc
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/hms/hms_catalog-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/auth_token_expire-itest.cc
M src/kudu/integration-tests/authz_token-itest.cc
M src/kudu/integration-tests/catalog_manager_tsk-itest.cc
M src/kudu/integration-tests/client-negotiation-failover-itest.cc
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/create-table-itest.cc
M src/kudu/integration-tests/delete_table-itest.cc
M src/kudu/integration-tests/disk_failure-itest.cc
M src/kudu/integration-tests/exactly_once_writes-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/raft_consensus-itest-base.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/security-faults-itest.cc
M src/kudu/integration-tests/security-itest.cc
M src/kudu/integration-tests/security-unknown-tsk-itest.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/tablet_replacement-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/integration-tests/ts_itest-base.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/integration-tests/txn_commit-itest.cc
M src/kudu/integration-tests/txn_participant-itest.cc
M src/kudu/integration-tests/txn_write_ops-itest.cc
M src/kudu/integration-tests/write_limit-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
M src/kudu/master/master.h
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/master/ts_state-test.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_applier.cc
M src/kudu/tablet/delta_applier.h
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/key_value_test_schema.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.cc
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_history_gc-test.cc
M src/kudu/tablet/tablet_metadata-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-base.cc
M src/kudu/tablet/tablet_replica-test-base.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_throttle-test.cc
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tools/create-table-tool-test.cc
M src/kudu/tools/ksck-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/ksck_remote.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_hms.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/tools/tool_action_table.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/transactions/txn_status_tablet.h
M src/kudu/transactions/txn_system_client.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
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-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_server_authorization-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
M src/kudu/util/subprocess.cc
185 files changed, 2,476 insertions(+), 2,111 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18098/3
-- 
To view, visit http://gerrit.cloudera.org:8080/18098
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
Gerrit-Change-Number: 18098
Gerrit-PatchSet: 3
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-3197 [tserver] optimal Schema's memory used

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
shenxingwuying@gmail.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/18098 )

Change subject: KUDU-3197 [tserver] optimal Schema's memory used
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18098/4//COMMIT_MSG
Commit Message:

PS4: 
> Started looking briefly into this, but it seems like a lot of the test-only
Yes, I agree with you. And that's my thought when start the work. But changes out of control.

If we use std::shared_ptr<Schema>, the changes will be less than scoped_refptr<Schema>.
And Kudu recommends scoped_refptr<Schema>.
Once we choose Schema extends RefCountedThreadSafe, the commits are the result, 
the changes are accumulated little by little. 

Another way is wangningito's way, that's a simple method but not strict, maybe small proability core.
wangningito's commit at: https://gerrit.cloudera.org/c/16508/


http://gerrit.cloudera.org:8080/#/c/18098/4/src/kudu/client/schema.h
File src/kudu/client/schema.h:

http://gerrit.cloudera.org:8080/#/c/18098/4/src/kudu/client/schema.h@732
PS4, Line 732:  /// Convert a KuduSchema to a Schema.
             :   ///
             :   /// Private API.
             :   ///
             :   /// @param[in] kudu_schema
             :   ///   The KuduSchema to convert
             :   /// @return The converted Schema
             :   static scoped_refptr<Schema> ToSchema(const KuduSchema& kudu_schema) KUDU_NO_EXPORT;
> This change doesn't seem necessary to address KUDU-3197
Yes. 

But Schema extends RefCountedThreadSafe, then the Schema's usage is changed. I have to change it.

Schema s;    is not valid because deconstruction will check fail. 
So Schema should use it like this  scoped_refptr<Schema> s(new Schema),  or use Schema&. 

The functions which return Schema would cause the problem when deconstruction.

A few functions below has changed like this.



-- 
To view, visit http://gerrit.cloudera.org:8080/18098
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
Gerrit-Change-Number: 18098
Gerrit-PatchSet: 5
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 22 Dec 2021 11:58:32 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3197 [tserver] optimal Schema's memory used

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18098 )

Change subject: KUDU-3197 [tserver] optimal Schema's memory used
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18098/4/src/kudu/client/schema.h
File src/kudu/client/schema.h:

http://gerrit.cloudera.org:8080/#/c/18098/4/src/kudu/client/schema.h@732
PS4, Line 732:  /// Convert a KuduSchema to a Schema.
             :   ///
             :   /// Private API.
             :   ///
             :   /// @param[in] kudu_schema
             :   ///   The KuduSchema to convert
             :   /// @return The converted Schema
             :   static scoped_refptr<Schema> ToSchema(const KuduSchema& kudu_schema) KUDU_NO_EXPORT;
> Yes. 
+1

I think std::shared_ptr is a good alternative in this context and it's used in Kudu codebase in many places.

If using std::shared_ptr, consider allocating it using std::make_shared().  BTW, there is a page providing more information on using std::shared_ptr vs scoped_refptr: https://kudu.apache.org/docs/contributing.html#_pointers



-- 
To view, visit http://gerrit.cloudera.org:8080/18098
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
Gerrit-Change-Number: 18098
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 22 Dec 2021 17:57:24 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3197 [tserver] optimal Schema's memory used

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18098 )

Change subject: KUDU-3197 [tserver] optimal Schema's memory used
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18098/4/src/kudu/client/schema.h
File src/kudu/client/schema.h:

http://gerrit.cloudera.org:8080/#/c/18098/4/src/kudu/client/schema.h@732
PS4, Line 732:  /// Convert a KuduSchema to a Schema.
             :   ///
             :   /// Private API.
             :   ///
             :   /// @param[in] kudu_schema
             :   ///   The KuduSchema to convert
             :   /// @return The converted Schema
             :   static scoped_refptr<Schema> ToSchema(const KuduSchema& kudu_schema) KUDU_NO_EXPORT;
> This change doesn't seem necessary to address KUDU-3197
Now class Schema is inherited from RefCountedThreadSafe<Schema>, constructor has been declared as 'delete', so we can't create such a Schema object after this patch, and that's why you can see so many changes, we can reduce changes by using std::shared_ptr, but that seems not the recommanded smart pointer in Kudu.



-- 
To view, visit http://gerrit.cloudera.org:8080/18098
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
Gerrit-Change-Number: 18098
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sun, 19 Dec 2021 14:49:53 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3197 [tserver] optimal Schema's memory used

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18098 )

Change subject: KUDU-3197 [tserver] optimal Schema's memory used
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18098/4/src/kudu/client/schema.h
File src/kudu/client/schema.h:

http://gerrit.cloudera.org:8080/#/c/18098/4/src/kudu/client/schema.h@732
PS4, Line 732:  /// Convert a KuduSchema to a Schema.
             :   ///
             :   /// Private API.
             :   ///
             :   /// @param[in] kudu_schema
             :   ///   The KuduSchema to convert
             :   /// @return The converted Schema
             :   static scoped_refptr<Schema> ToSchema(const KuduSchema& kudu_schema) KUDU_NO_EXPORT;
> Now class Schema is inherited from RefCountedThreadSafe<Schema>, constructo
I see. Typically the decision between scoped_refptr and shared_ptr is situational. I think in this case, it's worth going the std::shared_ptr route, given it allows us to allocate on the stack, on top of it likely being a significantly smaller change.



-- 
To view, visit http://gerrit.cloudera.org:8080/18098
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
Gerrit-Change-Number: 18098
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 22 Dec 2021 04:09:51 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3197 [tserver] optimal Schema's memory used

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has abandoned this change. ( http://gerrit.cloudera.org:8080/18098 )

Change subject: KUDU-3197 [tserver] optimal Schema's memory used
......................................................................


Abandoned

the schema use scoped_refptr<Schema> leads too much changes, peers advice use std::shared_ptr, and the work at https://gerrit.cloudera.org/c/18198/(WIP abandon) and then https://gerrit.cloudera.org/c/18255/
-- 
To view, visit http://gerrit.cloudera.org:8080/18098
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
Gerrit-Change-Number: 18098
Gerrit-PatchSet: 5
Gerrit-Owner: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>

[kudu-CR] KUDU-3197 [tserver] optimal Schema's memory used

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18098

to look at the new patch set (#4).

Change subject: KUDU-3197 [tserver] optimal Schema's memory used
......................................................................

KUDU-3197 [tserver] optimal Schema's memory used

change TabletMeta's variable Schema* to scoped_refptr<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 thought looks like simple, but implements is a
little complex, for Schema has changed to scoped_refptr<Schema>, leading to
some writes not correct before, I must have to repaire a lot of unit tests
and some others codes.

Conflicts:
	src/kudu/cfile/cfile-test.cc
	src/kudu/client/client-internal.cc
	src/kudu/client/client.cc
	src/kudu/client/meta_cache.cc
	src/kudu/client/scan_configuration.h
	src/kudu/client/scan_configuration.cc
	src/kudu/client/scan_token-internal.cc
	src/kudu/client/schema.cc
	src/kudu/common/encoded_key-test.cc
	src/kudu/common/generic_iterators.cc
	src/kudu/common/partition-test.cc
	src/kudu/common/partition.cc
	src/kudu/common/partition.h
	src/kudu/common/partition_pruner-test.cc
	src/kudu/common/row_operations-test.cc
	src/kudu/common/scan_spec-test.cc
	src/kudu/common/wire_protocol-test.cc
	src/kudu/hms/hms_catalog-test.cc
	src/kudu/integration-tests/fuzz-itest.cc
	src/kudu/integration-tests/master_hms-itest.cc
	src/kudu/integration-tests/security-itest.cc
	src/kudu/integration-tests/table_locations-itest.cc
	src/kudu/integration-tests/tablet_replacement-itest.cc
	src/kudu/integration-tests/txn_participant-itest.cc
	src/kudu/master/catalog_manager.cc
	src/kudu/master/catalog_manager.h
	src/kudu/master/master-test.cc
	src/kudu/master/master.cc
	src/kudu/master/master.h
	src/kudu/master/sentry_authz_provider-test.cc
	src/kudu/master/sys_catalog.cc
	src/kudu/tablet/all_types-scan-correctness-test.cc
	src/kudu/tablet/cfile_set-test.cc
	src/kudu/tablet/cfile_set.cc
	src/kudu/tablet/compaction-test.cc
	src/kudu/tablet/compaction.cc
	src/kudu/tablet/delta_compaction.cc
	src/kudu/tablet/deltafile-test.cc
	src/kudu/tablet/delta_store.h
	src/kudu/tablet/deltamemstore-test.cc
	src/kudu/tablet/diskrowset-test-base.h
	src/kudu/tablet/diskrowset.cc
	src/kudu/tablet/memrowset-test.cc
	src/kudu/tablet/memrowset.cc
	src/kudu/tablet/memrowset.h
	src/kudu/tablet/mock-rowsets.h
	src/kudu/tablet/mt-rowset_delta_compaction-test.cc
	src/kudu/tablet/mt-tablet-test.cc
	src/kudu/tablet/ops/alter_schema_op.h
	src/kudu/tablet/ops/alter_schema_op.cc
	src/kudu/tablet/tablet-decoder-eval-test.cc
	src/kudu/tablet/tablet-pushdown-test.cc
	src/kudu/tablet/tablet-test-base.h
	src/kudu/tablet/tablet-test-util.h
	src/kudu/tablet/tablet-test.cc
	src/kudu/tablet/tablet.cc
	src/kudu/tablet/tablet_bootstrap.cc
	src/kudu/tablet/tablet_metadata.cc
	src/kudu/tablet/tablet_random_access-test.cc
	src/kudu/tablet/tablet_replica-test.cc
	src/kudu/tablet/tablet-test-base.h
	src/kudu/tablet/tablet_metadata.cc
	src/kudu/tablet/tablet_metadata.h
	src/kudu/tools/master_rebuilder.cc
	src/kudu/tools/ksck-test.cc
	src/kudu/tools/ksck_remote.cc
	src/kudu/tools/kudu-tool-test.cc
	src/kudu/tools/table_scanner.cc
	src/kudu/tools/tool_action_fs.cc
	src/kudu/tools/tool_action_hms.cc
	src/kudu/tools/tool_action_master.cc
	src/kudu/tools/tool_action_perf.cc
	src/kudu/tools/tool_action_table.cc
	src/kudu/transactions/txn_system_client.cc
	src/kudu/tserver/scanners.cc
	src/kudu/tserver/scanners.h
	src/kudu/tserver/tablet_copy_client.cc
	src/kudu/tserver/tablet_server-test-base.cc
	src/kudu/tserver/tablet_server-test.cc
	src/kudu/tserver/tablet_server_authorization-test.cc
	src/kudu/tserver/tablet_service.cc
	src/kudu/tserver/tablet_server_authorization-test.cc
	src/kudu/tserver/ts_tablet_manager-test.cc

Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/client/batcher.cc
M src/kudu/client/client-internal.cc
M src/kudu/client/client-test.cc
M src/kudu/client/client-unittest.cc
M src/kudu/client/client.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/scan_configuration.cc
M src/kudu/client/scan_configuration.h
M src/kudu/client/scan_token-internal.cc
M src/kudu/client/scanner-internal.cc
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/client/table-internal.h
M src/kudu/client/write_op.cc
M src/kudu/codegen/codegen-test.cc
M src/kudu/codegen/compilation_manager.cc
M src/kudu/codegen/row_projector.cc
M src/kudu/codegen/row_projector.h
M src/kudu/common/encoded_key-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/iterator.h
M src/kudu/common/key_util-test.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/row_operations-test.cc
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/schema-test.cc
M src/kudu/common/schema.cc
M src/kudu/common/schema.h
M src/kudu/common/wire_protocol-test-util.h
M src/kudu/common/wire_protocol-test.cc
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-test.cc
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/hms/hms_catalog-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/auth_token_expire-itest.cc
M src/kudu/integration-tests/authz_token-itest.cc
M src/kudu/integration-tests/catalog_manager_tsk-itest.cc
M src/kudu/integration-tests/client-negotiation-failover-itest.cc
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/create-table-itest.cc
M src/kudu/integration-tests/delete_table-itest.cc
M src/kudu/integration-tests/disk_failure-itest.cc
M src/kudu/integration-tests/exactly_once_writes-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/raft_consensus-itest-base.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/security-faults-itest.cc
M src/kudu/integration-tests/security-itest.cc
M src/kudu/integration-tests/security-unknown-tsk-itest.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/tablet_replacement-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/integration-tests/ts_itest-base.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/integration-tests/txn_commit-itest.cc
M src/kudu/integration-tests/txn_participant-itest.cc
M src/kudu/integration-tests/txn_write_ops-itest.cc
M src/kudu/integration-tests/write_limit-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
M src/kudu/master/master.h
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/master/ts_state-test.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_applier.cc
M src/kudu/tablet/delta_applier.h
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/key_value_test_schema.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.cc
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_history_gc-test.cc
M src/kudu/tablet/tablet_metadata-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-base.cc
M src/kudu/tablet/tablet_replica-test-base.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_throttle-test.cc
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tools/create-table-tool-test.cc
M src/kudu/tools/ksck-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/ksck_remote.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_hms.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/tools/tool_action_table.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/transactions/txn_status_tablet.h
M src/kudu/transactions/txn_system_client.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
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-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_server_authorization-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
M src/kudu/util/subprocess.cc
185 files changed, 2,482 insertions(+), 2,114 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18098/4
-- 
To view, visit http://gerrit.cloudera.org:8080/18098
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
Gerrit-Change-Number: 18098
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-3197 [tserver] optimal Schema's memory used

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18098

to look at the new patch set (#5).

Change subject: KUDU-3197 [tserver] optimal Schema's memory used
......................................................................

KUDU-3197 [tserver] optimal Schema's memory used

change TabletMeta's variable Schema* to scoped_refptr<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 thought looks like simple, but implements is a
little complex, for Schema has changed to scoped_refptr<Schema>, leading to
some writes not correct before, I must have to repaire a lot of unit tests
and some others codes.

Conflicts:
	src/kudu/cfile/cfile-test.cc
	src/kudu/client/client-internal.cc
	src/kudu/client/client.cc
	src/kudu/client/meta_cache.cc
	src/kudu/client/scan_configuration.h
	src/kudu/client/scan_configuration.cc
	src/kudu/client/scan_token-internal.cc
	src/kudu/client/schema.cc
	src/kudu/common/encoded_key-test.cc
	src/kudu/common/generic_iterators.cc
	src/kudu/common/partition-test.cc
	src/kudu/common/partition.cc
	src/kudu/common/partition.h
	src/kudu/common/partition_pruner-test.cc
	src/kudu/common/row_operations-test.cc
	src/kudu/common/scan_spec-test.cc
	src/kudu/common/wire_protocol-test.cc
	src/kudu/hms/hms_catalog-test.cc
	src/kudu/integration-tests/fuzz-itest.cc
	src/kudu/integration-tests/master_hms-itest.cc
	src/kudu/integration-tests/security-itest.cc
	src/kudu/integration-tests/table_locations-itest.cc
	src/kudu/integration-tests/tablet_replacement-itest.cc
	src/kudu/integration-tests/txn_participant-itest.cc
	src/kudu/master/catalog_manager.cc
	src/kudu/master/catalog_manager.h
	src/kudu/master/master-test.cc
	src/kudu/master/master.cc
	src/kudu/master/master.h
	src/kudu/master/sentry_authz_provider-test.cc
	src/kudu/master/sys_catalog.cc
	src/kudu/tablet/all_types-scan-correctness-test.cc
	src/kudu/tablet/cfile_set-test.cc
	src/kudu/tablet/cfile_set.cc
	src/kudu/tablet/compaction-test.cc
	src/kudu/tablet/compaction.cc
	src/kudu/tablet/delta_compaction.cc
	src/kudu/tablet/deltafile-test.cc
	src/kudu/tablet/delta_store.h
	src/kudu/tablet/deltamemstore-test.cc
	src/kudu/tablet/diskrowset-test-base.h
	src/kudu/tablet/diskrowset.cc
	src/kudu/tablet/memrowset-test.cc
	src/kudu/tablet/memrowset.cc
	src/kudu/tablet/memrowset.h
	src/kudu/tablet/mock-rowsets.h
	src/kudu/tablet/mt-rowset_delta_compaction-test.cc
	src/kudu/tablet/mt-tablet-test.cc
	src/kudu/tablet/ops/alter_schema_op.h
	src/kudu/tablet/ops/alter_schema_op.cc
	src/kudu/tablet/tablet-decoder-eval-test.cc
	src/kudu/tablet/tablet-pushdown-test.cc
	src/kudu/tablet/tablet-test-base.h
	src/kudu/tablet/tablet-test-util.h
	src/kudu/tablet/tablet-test.cc
	src/kudu/tablet/tablet.cc
	src/kudu/tablet/tablet_bootstrap.cc
	src/kudu/tablet/tablet_metadata.cc
	src/kudu/tablet/tablet_random_access-test.cc
	src/kudu/tablet/tablet_replica-test.cc
	src/kudu/tablet/tablet-test-base.h
	src/kudu/tablet/tablet_metadata.cc
	src/kudu/tablet/tablet_metadata.h
	src/kudu/tools/master_rebuilder.cc
	src/kudu/tools/ksck-test.cc
	src/kudu/tools/ksck_remote.cc
	src/kudu/tools/kudu-tool-test.cc
	src/kudu/tools/table_scanner.cc
	src/kudu/tools/tool_action_fs.cc
	src/kudu/tools/tool_action_hms.cc
	src/kudu/tools/tool_action_master.cc
	src/kudu/tools/tool_action_perf.cc
	src/kudu/tools/tool_action_table.cc
	src/kudu/transactions/txn_system_client.cc
	src/kudu/tserver/scanners.cc
	src/kudu/tserver/scanners.h
	src/kudu/tserver/tablet_copy_client.cc
	src/kudu/tserver/tablet_server-test-base.cc
	src/kudu/tserver/tablet_server-test.cc
	src/kudu/tserver/tablet_server_authorization-test.cc
	src/kudu/tserver/tablet_service.cc
	src/kudu/tserver/tablet_server_authorization-test.cc
	src/kudu/tserver/ts_tablet_manager-test.cc

Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/client/batcher.cc
M src/kudu/client/client-internal.cc
M src/kudu/client/client-test.cc
M src/kudu/client/client-unittest.cc
M src/kudu/client/client.cc
M src/kudu/client/meta_cache.cc
M src/kudu/client/scan_configuration.cc
M src/kudu/client/scan_configuration.h
M src/kudu/client/scan_token-internal.cc
M src/kudu/client/scanner-internal.cc
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/client/table-internal.h
M src/kudu/client/write_op.cc
M src/kudu/codegen/codegen-test.cc
M src/kudu/codegen/compilation_manager.cc
M src/kudu/codegen/row_projector.cc
M src/kudu/codegen/row_projector.h
M src/kudu/common/encoded_key-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/iterator.h
M src/kudu/common/key_util-test.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/row_operations-test.cc
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/schema-test.cc
M src/kudu/common/schema.cc
M src/kudu/common/schema.h
M src/kudu/common/wire_protocol-test-util.h
M src/kudu/common/wire_protocol-test.cc
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-test.cc
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/hms/hms_catalog-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/auth_token_expire-itest.cc
M src/kudu/integration-tests/authz_token-itest.cc
M src/kudu/integration-tests/catalog_manager_tsk-itest.cc
M src/kudu/integration-tests/client-negotiation-failover-itest.cc
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/create-table-itest.cc
M src/kudu/integration-tests/delete_table-itest.cc
M src/kudu/integration-tests/disk_failure-itest.cc
M src/kudu/integration-tests/exactly_once_writes-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/raft_consensus-itest-base.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/security-faults-itest.cc
M src/kudu/integration-tests/security-itest.cc
M src/kudu/integration-tests/security-unknown-tsk-itest.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/tablet_replacement-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/integration-tests/ts_itest-base.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/integration-tests/txn_commit-itest.cc
M src/kudu/integration-tests/txn_participant-itest.cc
M src/kudu/integration-tests/txn_write_ops-itest.cc
M src/kudu/integration-tests/write_limit-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
M src/kudu/master/master.h
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/master/ts_state-test.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_applier.cc
M src/kudu/tablet/delta_applier.h
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/key_value_test_schema.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.cc
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_history_gc-test.cc
M src/kudu/tablet/tablet_metadata-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-base.cc
M src/kudu/tablet/tablet_replica-test-base.h
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_throttle-test.cc
M src/kudu/tablet/txn_participant-test.cc
M src/kudu/tools/create-table-tool-test.cc
M src/kudu/tools/ksck-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/ksck_remote.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/master_rebuilder.cc
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/table_scanner.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_hms.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/tools/tool_action_table.cc
M src/kudu/transactions/txn_status_tablet.cc
M src/kudu/transactions/txn_status_tablet.h
M src/kudu/transactions/txn_system_client.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
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-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_server_authorization-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
M src/kudu/util/subprocess.cc
185 files changed, 2,482 insertions(+), 2,114 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/18098/5
-- 
To view, visit http://gerrit.cloudera.org:8080/18098
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
Gerrit-Change-Number: 18098
Gerrit-PatchSet: 5
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3197 [tserver] optimal Schema's memory used

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18098 )

Change subject: KUDU-3197 [tserver] optimal Schema's memory used
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18098/4/src/kudu/client/schema.h
File src/kudu/client/schema.h:

http://gerrit.cloudera.org:8080/#/c/18098/4/src/kudu/client/schema.h@732
PS4, Line 732:  /// Convert a KuduSchema to a Schema.
             :   ///
             :   /// Private API.
             :   ///
             :   /// @param[in] kudu_schema
             :   ///   The KuduSchema to convert
             :   /// @return The converted Schema
             :   static scoped_refptr<Schema> ToSchema(const KuduSchema& kudu_schema) KUDU_NO_EXPORT;
> +1
Just to clarify on my earlier comment: using std::shared_ptr instead of scoped_refptr is a good alternative.

However, changing the return type of the KuduSchema::ToSchema(const KuduSchema&) method would break the ABI compatibility for Kudu C++ client, so this particular update isn't acceptable.  See https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B for details on the ABI compatibility.



-- 
To view, visit http://gerrit.cloudera.org:8080/18098
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
Gerrit-Change-Number: 18098
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 22 Dec 2021 18:04:15 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3197 [tserver] optimal Schema's memory used

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18098 )

Change subject: KUDU-3197 [tserver] optimal Schema's memory used
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/18098/4//COMMIT_MSG
Commit Message:

PS4: 
Started looking briefly into this, but it seems like a lot of the test-only and client-side usages of Schemas were fine as is. I think we should limit the scope of this change to only worry about the Schemas in old_schemas_ in TabletMetadata, since that's what was consuming memory in KUDU-3197.


http://gerrit.cloudera.org:8080/#/c/18098/4/src/kudu/client/schema.h
File src/kudu/client/schema.h:

http://gerrit.cloudera.org:8080/#/c/18098/4/src/kudu/client/schema.h@732
PS4, Line 732:  /// Convert a KuduSchema to a Schema.
             :   ///
             :   /// Private API.
             :   ///
             :   /// @param[in] kudu_schema
             :   ///   The KuduSchema to convert
             :   /// @return The converted Schema
             :   static scoped_refptr<Schema> ToSchema(const KuduSchema& kudu_schema) KUDU_NO_EXPORT;
This change doesn't seem necessary to address KUDU-3197



-- 
To view, visit http://gerrit.cloudera.org:8080/18098
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c9cf24475de0b787542761522915a86db6564d7
Gerrit-Change-Number: 18098
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 14 Dec 2021 20:35:13 +0000
Gerrit-HasComments: Yes