You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by bgaff <gi...@git.apache.org> on 2015/07/09 04:50:05 UTC
[GitHub] trafficserver pull request: Add ListAll for HostDB httpui endpoint
GitHub user bgaff opened a pull request:
https://github.com/apache/trafficserver/pull/245
Add ListAll for HostDB httpui endpoint
We would like a way to enumerate all records in hostdb (this doesn't include the hostsfile map, although adding that would be a trivial change). I really would like feedback and suggestions.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bgaff/trafficserver master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/245.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #245
----
commit 93666d15466c4d8b29e52f1cbfab0504fd3f34c2
Author: Brian Geffon <br...@apache.org>
Date: 2015-07-09T02:48:45Z
Add ListAll for HostDB httpui endpoint
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: Add ListAll for HostDB httpui endpoint
Posted by bgaff <gi...@git.apache.org>.
Github user bgaff commented on the pull request:
https://github.com/apache/trafficserver/pull/245#issuecomment-120851447
All, I've incorporated @SolidWallOfCode 's feedback which renames the method to iterate / iterateEvent, additionally i've modified the code to reschedule if it's unable to get a bucket's mutex and after 100 buckets (my local build had around 3700) so that seemed like a reasonable number, the reschedule interval for normal events is 5ms, if you can't get the mutex it uses the current 20ms timeout. Any other comments before I merge?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: Add ListAll for HostDB httpui endpoint
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on the pull request:
https://github.com/apache/trafficserver/pull/245#issuecomment-120191769
@jpeach from our conversations on IRC it sounds like you aren't opposed to adding this, but rather want to start making this more general. If we could make a ticket for the generic version we could start adding in all of the endpoints that we use from these and migrate them to a more unified API.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: Add ListAll for HostDB httpui endpoint
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the pull request:
https://github.com/apache/trafficserver/pull/245#issuecomment-120151397
Because generating raw HTML in core code is unmaintainable. The right approach is to extract the data and publish it in the appropriate format for the consumer (e.g. JSON or whatever).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: Add ListAll for HostDB httpui endpoint
Posted by bgaff <gi...@git.apache.org>.
Github user bgaff commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/245#discussion_r34431846
--- Diff: iocore/hostdb/HostDB.cc ---
@@ -1801,6 +1828,39 @@ HostDBContinuation::do_put_response(ClusterMachine *m, HostDBInfo *r, Continuati
clusterProcessor.invoke_remote(m->pop_ClusterHandler(), PUT_HOSTINFO_CLUSTER_FUNCTION, (char *)&msg, len);
}
+int
+HostDBContinuation::probeAllEvent(int event, Event *e)
+{
+ Debug("hostdb", "probeAllEvent event=%d eventp=%p", event, e);
+ ink_assert(!link.prev && !link.next);
+ EThread *t = e ? e->ethread : this_ethread();
+
+ MUTEX_TRY_LOCK_FOR(lock, action.mutex, t, action.continuation);
+ if (!lock.is_locked()) {
+ mutex->thread_holding->schedule_in(this, HOST_DB_RETRY_PERIOD);
+ return EVENT_CONT;
+ }
+
+ if (action.cancelled) {
+ hostdb_cont_free(this);
+ return EVENT_DONE;
+ }
+
+ for (int i = 0; i < hostDB.buckets; ++i) {
+ ProxyMutex *bucket_mutex = hostDB.lock_for_bucket(i);
+ SCOPED_MUTEX_LOCK(lock, bucket_mutex, t);
+ for (unsigned int l = 0; l < hostDB.levels; ++l) {
+ HostDBInfo *r = reinterpret_cast<HostDBInfo*>(hostDB.data + hostDB.level_offset[l] + hostDB.bucketsize[l] * i);
+ if (!r->deleted && !r->failed()) {
+ action.continuation->handleEvent(EVENT_INTERVAL, static_cast<void*>(r));
+ }
+ }
+ }
+
--- End diff --
I agree, I'll add rescheduling after iterating a few buckets.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: Add ListAll for HostDB httpui endpoint
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on the pull request:
https://github.com/apache/trafficserver/pull/245#issuecomment-120147643
Why do we want to kill the httpui? We use it all the time for stats collection
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: Add ListAll for HostDB httpui endpoint
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the pull request:
https://github.com/apache/trafficserver/pull/245#issuecomment-120039117
I'm strongly in favor of introspecting HostDB, but I don't like extending HTTPUI. The right way to do this IMHO is to add management APIs so you can expose it through multiple layers (e.g. traffic_ctl). You can also add lookup and purge operations as well as list/dump.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: Add ListAll for HostDB httpui endpoint
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on the pull request:
https://github.com/apache/trafficserver/pull/245#issuecomment-120191816
@SolidWallOfCode How does this PR look to you?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: Add ListAll for HostDB httpui endpoint
Posted by bgaff <gi...@git.apache.org>.
Github user bgaff commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/245#discussion_r34431849
--- Diff: iocore/hostdb/HostDB.cc ---
@@ -1037,6 +1037,33 @@ HostDBProcessor::getbyname_imm(Continuation *cont, process_hostdb_info_pfn proce
return &c->action;
}
+Action *
+HostDBProcessor::getall(Continuation *cont)
--- End diff --
Sure, name change incoming.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: Add ListAll for HostDB httpui endpoint
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the pull request:
https://github.com/apache/trafficserver/pull/245#issuecomment-120083151
Absolutely agree, lets not put more stuff into httpui, we should aim to eliminate it for 7.0.0. Move things over to the management port and traffic_ctl please.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: Add ListAll for HostDB httpui endpoint
Posted by bgaff <gi...@git.apache.org>.
Github user bgaff closed the pull request at:
https://github.com/apache/trafficserver/pull/245
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: Add ListAll for HostDB httpui endpoint
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/245#discussion_r34371196
--- Diff: iocore/hostdb/HostDB.cc ---
@@ -1801,6 +1828,39 @@ HostDBContinuation::do_put_response(ClusterMachine *m, HostDBInfo *r, Continuati
clusterProcessor.invoke_remote(m->pop_ClusterHandler(), PUT_HOSTINFO_CLUSTER_FUNCTION, (char *)&msg, len);
}
+int
+HostDBContinuation::probeAllEvent(int event, Event *e)
+{
+ Debug("hostdb", "probeAllEvent event=%d eventp=%p", event, e);
+ ink_assert(!link.prev && !link.next);
+ EThread *t = e ? e->ethread : this_ethread();
+
+ MUTEX_TRY_LOCK_FOR(lock, action.mutex, t, action.continuation);
+ if (!lock.is_locked()) {
+ mutex->thread_holding->schedule_in(this, HOST_DB_RETRY_PERIOD);
+ return EVENT_CONT;
+ }
+
+ if (action.cancelled) {
+ hostdb_cont_free(this);
+ return EVENT_DONE;
+ }
+
+ for (int i = 0; i < hostDB.buckets; ++i) {
+ ProxyMutex *bucket_mutex = hostDB.lock_for_bucket(i);
+ SCOPED_MUTEX_LOCK(lock, bucket_mutex, t);
+ for (unsigned int l = 0; l < hostDB.levels; ++l) {
+ HostDBInfo *r = reinterpret_cast<HostDBInfo*>(hostDB.data + hostDB.level_offset[l] + hostDB.bucketsize[l] * i);
+ if (!r->deleted && !r->failed()) {
+ action.continuation->handleEvent(EVENT_INTERVAL, static_cast<void*>(r));
+ }
+ }
+ }
+
--- End diff --
Can you considered doing 1 or some small finite number of buckets per event? Otherwise you're blocking a thread for potentially a long time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: Add ListAll for HostDB httpui endpoint
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/245#discussion_r34371142
--- Diff: iocore/hostdb/HostDB.cc ---
@@ -1037,6 +1037,33 @@ HostDBProcessor::getbyname_imm(Continuation *cont, process_hostdb_info_pfn proce
return &c->action;
}
+Action *
+HostDBProcessor::getall(Continuation *cont)
--- End diff --
Bad name. This should be "iterate" because that's what it is doing or "probeAll" to be consistent with the event / method name.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---