On Friday, February 01, 2013 04:36:37 PM jpe...@apache.org wrote: > Updated Branches: > refs/heads/master baca4d2b7 -> 5f93bc50f > > > TS-1678: Simplify register_record() > > 'release_record_lock' parameter to register_record() is unnecessarily > complicated and the lock release is dangerous. Let's simplify it
I'm all for removing code but would you care to explain why "the lock release is dangerous" ? -- i > and remove verbose lock releasing from the upper layer code. > > > Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo > Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/5f93bc50 > Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/5f93bc50 > Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/5f93bc50 > > Branch: refs/heads/master > Commit: 5f93bc50fd1869f4eb2866ecffd92fc616835fb4 > Parents: baca4d2 > Author: Yunkai Zhang <qiushu....@taobao.com> > Authored: Fri Feb 1 08:35:07 2013 -0800 > Committer: James Peach <jpe...@apache.org> > Committed: Fri Feb 1 08:35:07 2013 -0800 > > ---------------------------------------------------------------------- > CHANGES | 3 +++ > lib/records/RecCore.cc | 18 +++--------------- > 2 files changed, 6 insertions(+), 15 deletions(-) > ---------------------------------------------------------------------- > > > http://git-wip-us.apache.org/repos/asf/trafficserver/blob/5f93bc50/CHANGES > ---------------------------------------------------------------------- > diff --git a/CHANGES b/CHANGES > index ad32f87..9da2ed4 100644 > --- a/CHANGES > +++ b/CHANGES > @@ -1,6 +1,9 @@ > -*- coding: utf-8 > -*- Changes with Apache Traffic Server 3.3.1 > > + *) [TS-1678] Simplify register_record > + Author: Yunkai Zhang <qiushu....@taobao.com> > + > *) [TS-1252] stats summary in cluster not working > Author: Yunkai Zhang <qiushu....@taobao.com> > > > http://git-wip-us.apache.org/repos/asf/trafficserver/blob/5f93bc50/lib/recor > ds/RecCore.cc > ---------------------------------------------------------------------- diff > --git a/lib/records/RecCore.cc b/lib/records/RecCore.cc > index d9215ed..4888fbb 100644 > --- a/lib/records/RecCore.cc > +++ b/lib/records/RecCore.cc > @@ -52,14 +52,11 @@ RecTree *g_records_tree = NULL; > // register_record > //------------------------------------------------------------------------- > static RecRecord * > -register_record(RecT rec_type, const char *name, RecDataT data_type, > RecData data_default, bool release_record_lock) +register_record(RecT > rec_type, const char *name, RecDataT data_type, RecData data_default) { > RecRecord *r = NULL; > > if (ink_hash_table_lookup(g_records_ht, name, (void **) &r)) { > - if (release_record_lock) { > - rec_mutex_acquire(&(r->lock)); > - } > ink_release_assert(r->rec_type == rec_type); > ink_release_assert(r->data_type == data_type); > // Note: do not set r->data as we want to keep the previous value > @@ -68,9 +65,6 @@ register_record(RecT rec_type, const char *name, RecDataT > data_type, RecData dat if ((r = RecAlloc(rec_type, name, data_type)) == > NULL) { > return NULL; > } > - if (release_record_lock) { > - rec_mutex_acquire(&(r->lock)); > - } > // Set the r->data to its default value as this is a new record > RecDataSet(r->data_type, &(r->data), &(data_default)); > RecDataSet(r->data_type, &(r->data_default), &(data_default)); > @@ -80,10 +74,6 @@ register_record(RecT rec_type, const char *name, RecDataT > data_type, RecData dat // we're now registered > r->registered = true; > > - if (release_record_lock) { > - rec_mutex_release(&(r->lock)); > - } > - > return r; > } > > @@ -733,9 +723,8 @@ RecRegisterStat(RecT rec_type, const char *name, > RecDataT data_type, RecData dat RecRecord *r = NULL; > > ink_rwlock_wrlock(&g_records_rwlock); > - if ((r = register_record(rec_type, name, data_type, data_default, false)) > != NULL) { + if ((r = register_record(rec_type, name, data_type, > data_default)) != NULL) { r->stat_meta.persist_type = persist_type; > - rec_mutex_release(&(r->lock)); > } else { > ink_debug_assert(!"Can't register record!"); > } > @@ -755,7 +744,7 @@ RecRegisterConfig(RecT rec_type, const char *name, > RecDataT data_type, { > RecRecord *r; > ink_rwlock_wrlock(&g_records_rwlock); > - if ((r = register_record(rec_type, name, data_type, data_default, false)) > != NULL) { + if ((r = register_record(rec_type, name, data_type, > data_default)) != NULL) { // Note: do not modify > 'record->config_meta.update_required' > r->config_meta.update_type = update_type; > r->config_meta.check_type = check_type; > @@ -765,7 +754,6 @@ RecRegisterConfig(RecT rec_type, const char *name, > RecDataT data_type, r->config_meta.check_expr = ats_strdup(check_expr); > r->config_meta.update_cb_list = NULL; > r->config_meta.access_type = access_type; > - rec_mutex_release(&(r->lock)); > } > ink_rwlock_unlock(&g_records_rwlock);
