You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by so...@apache.org on 2013/05/16 18:45:11 UTC
git commit: TS-1728: Assign storage entries to volumes
Updated Branches:
refs/heads/issues/TS-1728 [created] 1d8179df5
TS-1728: Assign storage entries to volumes
Introduces the volume assignment in storage.config, on an optional
basis. No change is needed in the config if you don't want to use
this feature.
* Add an optional reference to the volume number in storage.config,
and the Span object, in Store.h and Store.cc
* Add the volume reference to DiskVol, CacheProcessor::start_internal
(in Cache.cc)
* Change cplist_reconfigure (in Cache.cc) to constrain mapping
* Fix bytes_used and percent_used per volume stats
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/1d8179df
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/1d8179df
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/1d8179df
Branch: refs/heads/issues/TS-1728
Commit: 1d8179df5bfbde4331ee94519c7a7a9d45f2ebe3
Parents: 8472c79
Author: Justin Laue <ju...@fp-x.com>
Authored: Thu May 16 10:36:30 2013 -0600
Committer: Phil Sorber <so...@apache.org>
Committed: Thu May 16 10:42:02 2013 -0600
----------------------------------------------------------------------
CHANGES | 3 +
iocore/cache/Cache.cc | 113 ++++++++++++++++++++++++++++++++-------
iocore/cache/I_Store.h | 1 +
iocore/cache/P_CacheDisk.h | 4 +-
iocore/cache/Store.cc | 31 ++++++++++-
5 files changed, 130 insertions(+), 22 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1d8179df/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index cf42d0d..da80cf7 100644
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,9 @@
Changes with Apache Traffic Server 3.3.3
+ *) [TS-1728] Assign storage entries to volumes
+ Author: Justin Laue <ju...@fp-x.com>
+
*) [TS-1899] strtod() does not honor Hex strings on Solaris.
*) [TS-1894] madvise() not found on Solaris. And some cleanup.
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1d8179df/iocore/cache/Cache.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/Cache.cc b/iocore/cache/Cache.cc
index fbb97f3..4c7b9e4 100644
--- a/iocore/cache/Cache.cc
+++ b/iocore/cache/Cache.cc
@@ -215,24 +215,16 @@ int64_t
cache_bytes_used(int volume)
{
uint64_t used = 0;
- int start = 0; // These defaults are for volume 0, or volume 1 with volume.config
- int end = 1;
- if (-1 == volume) {
- end = gnvol;
- } else if (volume > 1) { // Special case when volume.config is used
- start = volume - 1;
- end = volume;
- }
-
- for (int i = start; i < end; i++) {
- if (!DISK_BAD(gvol[i]->disk)) {
+ for (int i = 0; i < gnvol; i++) {
+ if (!DISK_BAD(gvol[i]->disk) && (volume == -1 || gvol[i]->cache_vol->vol_number == volume)) {
if (!gvol[i]->header->cycle)
used += gvol[i]->header->write_pos - gvol[i]->start;
else
used += gvol[i]->len - vol_dirlen(gvol[i]) - EVACUATION_SIZE;
}
}
+
return used;
}
@@ -240,18 +232,29 @@ int
cache_stats_bytes_used_cb(const char *name, RecDataT data_type, RecData *data, RecRawStatBlock *rsb, int id)
{
int volume = -1;
+ char *p;
NOWARN_UNUSED(data_type);
NOWARN_UNUSED(data);
// Well, there's no way to pass along the volume ID, so extracting it from the stat name.
- if (0 == strncmp(name+20, "volume_", 10))
- volume = strtol(name+30, NULL, 10);
-
- if (cacheProcessor.initialized == CACHE_INITIALIZED)
- RecSetGlobalRawStatSum(rsb, id, cache_bytes_used(volume));
+ p = strstr((char *) name, "volume_");
+ if (p != NULL) {
+ // I'm counting on the compiler to optimize out strlen("volume_").
+ volume = strtol(p + strlen("volume_"), NULL, 10);
+ }
- RecRawStatSyncSum(name, data_type, data, rsb, id);
+ if (cacheProcessor.initialized == CACHE_INITIALIZED) {
+ int64_t used, total =0;
+ float percent_full;
+ used = cache_bytes_used(volume);
+ RecSetGlobalRawStatSum(rsb, id, used);
+ RecRawStatSyncSum(name, data_type, data, rsb, id);
+ RecGetGlobalRawStatSum(rsb, (int) cache_bytes_total_stat, &total);
+ percent_full = (float)used / (float)total * 100;
+ // The perent_full float below gets rounded down
+ RecSetGlobalRawStatSum(rsb, (int) cache_percent_full_stat, percent_full);
+ }
return 1;
}
@@ -645,6 +648,7 @@ CacheProcessor::start_internal(int flags)
}
if (diskok) {
gdisks[gndisks] = NEW(new CacheDisk());
+ gdisks[gndisks]->forced_volume_num = sd->vol_num;
Debug("cache_hosting", "Disk: %d, blocks: %d", gndisks, blocks);
int sector_size = sd->hw_sector_size;
@@ -2336,11 +2340,19 @@ cplist_update()
} else {
/* delete this volume from all the disks */
int d_no;
+ int clearCV = 1;
for (d_no = 0; d_no < gndisks; d_no++) {
- if (cp->disk_vols[d_no])
- cp->disk_vols[d_no]->disk->delete_volume(cp->vol_number);
+ if (cp->disk_vols[d_no]) {
+ if(cp->disk_vols[d_no]->disk->forced_volume_num == cp->vol_number) {
+ clearCV = 0;
+ config_vol->cachep = cp;
+ } else {
+ cp->disk_vols[d_no]->disk->delete_volume(cp->vol_number);
+ }
+ }
}
- config_vol = NULL;
+ if (clearCV)
+ config_vol = NULL;
}
break;
}
@@ -2365,6 +2377,46 @@ cplist_update()
}
}
+int fillExclusiveDisks(CacheVol *cp) {
+ int diskCount = 0;
+ int volume_number = cp->vol_number;
+ Debug("cache_init", "volume %d", volume_number);
+
+ for (int i = 0; i < gndisks; i++) {
+ if(gdisks[i]->forced_volume_num != volume_number)
+ continue;
+ /* The user had created several volumes before - clear the disk
+ and create one volume for http */
+ for(int j = 0; j < (int)gdisks[i]->header->num_volumes; j++) {
+ if (volume_number != gdisks[i]->disk_vols[j]->vol_number) {
+ Note("Clearing Disk: %s", gdisks[i]->path);
+ gdisks[i]->delete_all_volumes();
+ break;
+ }
+ }
+
+ diskCount++;
+ int64_t size_diff = gdisks[i]->num_usable_blocks;
+ DiskVolBlock *dpb;
+ do {
+ dpb = gdisks[i]->create_volume(volume_number, size_diff, cp->scheme);
+ if (dpb) {
+ if (!cp->disk_vols[i]) {
+ cp->disk_vols[i] = gdisks[i]->get_diskvol(volume_number);
+ }
+ size_diff -= dpb->len;
+ cp->size += dpb->len;
+ cp->num_vols++;
+ } else {
+ Debug("cache_init", "create_volume failed");
+ break;
+ }
+ } while ((size_diff > 0));
+ }
+ return diskCount;
+}
+
+
int
cplist_reconfigure()
{
@@ -2456,7 +2508,16 @@ cplist_reconfigure()
/* go through volume config and grow and create volumes */
config_vol = config_volumes.cp_queue.head;
+ for (; config_vol; config_vol = config_vol->link.next) {
+ // if volume is given exclusive disks, fill here and continue
+ volume_number = config_vol->number;
+ if (!config_vol->cachep) {
+ continue;
+ }
+ fillExclusiveDisks(config_vol->cachep);
+ }
+ config_vol = config_volumes.cp_queue.head;
for (; config_vol; config_vol = config_vol->link.next) {
size = config_vol->size;
@@ -2467,6 +2528,11 @@ cplist_reconfigure()
size_in_blocks = ((off_t) size * 1024 * 1024) / STORE_BLOCK_SIZE;
+ if(config_vol->cachep && config_vol->cachep->num_vols > 0) {
+ gnvol += config_vol->cachep->num_vols;
+ continue;
+ }
+
if (!config_vol->cachep) {
// we did not find a corresponding entry in cache vol...creat one
@@ -2578,6 +2644,13 @@ create_volume(int volume_number, off_t size_in_blocks, int scheme, CacheVol *cp)
off_t blocks_per_vol = VOL_BLOCK_SIZE >> STORE_BLOCK_SHIFT;
int full_disks = 0;
+ cp->vol_number = volume_number;
+ cp->scheme = scheme;
+ if(fillExclusiveDisks(cp)) {
+ Debug("cache_init", "volume successfully filled from forced disks: volume_number=%d", volume_number);
+ return 0;
+ }
+
int *sp = new int[gndisks];
memset(sp, 0, gndisks * sizeof(int));
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1d8179df/iocore/cache/I_Store.h
----------------------------------------------------------------------
diff --git a/iocore/cache/I_Store.h b/iocore/cache/I_Store.h
index 0188b1d..ebc3c5f 100644
--- a/iocore/cache/I_Store.h
+++ b/iocore/cache/I_Store.h
@@ -51,6 +51,7 @@ struct Span
int64_t offset; // used only if (file == true)
int alignment;
int disk_id;
+ int vol_num;
LINK(Span, link);
private:
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1d8179df/iocore/cache/P_CacheDisk.h
----------------------------------------------------------------------
diff --git a/iocore/cache/P_CacheDisk.h b/iocore/cache/P_CacheDisk.h
index 60379be..e5133d5 100644
--- a/iocore/cache/P_CacheDisk.h
+++ b/iocore/cache/P_CacheDisk.h
@@ -103,12 +103,14 @@ struct CacheDisk: public Continuation
DiskVol *free_blocks;
int num_errors;
int cleared;
+ int forced_volume_num; // assuming zero is not a valid volume number
CacheDisk()
: Continuation(new_ProxyMutex()), header(NULL),
path(NULL), header_len(0), len(0), start(0), skip(0),
num_usable_blocks(0), fd(-1), free_space(0), wasted_space(0),
- disk_vols(NULL), free_blocks(NULL), num_errors(0), cleared(0)
+ disk_vols(NULL), free_blocks(NULL), num_errors(0), cleared(0),
+ forced_volume_num(0)
{ }
~CacheDisk();
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1d8179df/iocore/cache/Store.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/Store.cc b/iocore/cache/Store.cc
index 9076dc1..c843cbb 100644
--- a/iocore/cache/Store.cc
+++ b/iocore/cache/Store.cc
@@ -28,6 +28,31 @@
// Global
Store theStore;
+#define VOL_STR "volume="
+int getVolume(char* line) {
+ int v = 0;
+ if(!line) return 0;
+ char* str = strstr(line, VOL_STR);
+ char* vol_start = str;
+ if(!str) return 0;
+ while (*str && !ParseRules::is_digit(*str))
+ str++;
+ v = ink_atoi(str);
+
+ while (*str && ParseRules::is_digit(*str))
+ str++;
+ while(*str) {
+ *vol_start = *str;
+ vol_start++;
+ str++;
+ }
+ *vol_start = 0;
+ Debug("cache_init", "returning %d and '%s'", v, line);
+
+ if(v < 0) return 0;
+ return v;
+}
+
//
// Store
@@ -291,6 +316,8 @@ Store::read_config(int fd)
if (!*n)
continue;
+ int volume_id = getVolume(n);
+
// parse
Debug("cache_init", "Store::read_config: \"%s\"", n);
@@ -310,7 +337,9 @@ Store::read_config(int fd)
n[len] = 0;
char *pp = Layout::get()->relative(n);
ns = NEW(new Span);
- Debug("cache_init", "Store::read_config - ns = NEW (new Span); ns->init(\"%s\",%" PRId64 ")", pp, size);
+ ns->vol_num = volume_id;
+ Debug("cache_init", "Store::read_config - ns = NEW (new Span); ns->init(\"%s\",%" PRId64 "), ns->vol_num=%d",
+ pp, size, ns->vol_num);
if ((err = ns->init(pp, size))) {
char buf[4096];
snprintf(buf, sizeof(buf), "could not initialize storage \"%s\" [%s]", pp, err);
Re: git commit: TS-1728: Assign storage entries to volumes
Posted by James Peach <ja...@me.com>.
On May 21, 2013, at 1:02 PM, Igor Galić <i....@brainsware.org> wrote:
>
>
> ----- Original Message -----
>> Updated Branches:
>> refs/heads/issues/TS-1728 [created] 1d8179df5
>>
>>
>> TS-1728: Assign storage entries to volumes
>>
>> Introduces the volume assignment in storage.config, on an optional
>> basis. No change is needed in the config if you don't want to use
>> this feature.
>>
>> * Add an optional reference to the volume number in storage.config,
>> and the Span object, in Store.h and Store.cc
>> * Add the volume reference to DiskVol, CacheProcessor::start_internal
>> (in Cache.cc)
>> * Change cplist_reconfigure (in Cache.cc) to constrain mapping
>> * Fix bytes_used and percent_used per volume stats
> [snip]
>
> n.b.: This codebase is too big/complex for *me* to review, the comments here
> are only things that I noticed while reading the diff on a train ride.
>
>> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1d8179df/iocore/cache/Cache.cc
>> ----------------------------------------------------------------------
>> diff --git a/iocore/cache/Cache.cc b/iocore/cache/Cache.cc
>> index fbb97f3..4c7b9e4 100644
>> --- a/iocore/cache/Cache.cc
>> +++ b/iocore/cache/Cache.cc
>
>> @@ -2456,7 +2508,16 @@ cplist_reconfigure()
>> /* go through volume config and grow and create volumes */
>>
>> config_vol = config_volumes.cp_queue.head;
>> + for (; config_vol; config_vol = config_vol->link.next) {
>
> form/consistency question: why not have config_vol = config_volumes.cp_queue.head as first part of the for loop?
>
>> + // if volume is given exclusive disks, fill here and continue
>> + volume_number = config_vol->number;
>> + if (!config_vol->cachep) {
>> + continue;
>> + }
>> + fillExclusiveDisks(config_vol->cachep);
>> + }
>>
>> + config_vol = config_volumes.cp_queue.head;
>> for (; config_vol; config_vol = config_vol->link.next) {
>>
>> size = config_vol->size;
>> @@ -2467,6 +2528,11 @@ cplist_reconfigure()
> [snip]
>> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1d8179df/iocore/cache/Store.cc
>> ----------------------------------------------------------------------
>> diff --git a/iocore/cache/Store.cc b/iocore/cache/Store.cc
>> index 9076dc1..c843cbb 100644
>> --- a/iocore/cache/Store.cc
>> +++ b/iocore/cache/Store.cc
>> @@ -28,6 +28,31 @@
>> // Global
>> Store theStore;
>>
>> +#define VOL_STR "volume="
>
> consider making this a C++ constant
>
>> +int getVolume(char* line) {
>
> consider defining this function static.
+1 on making everything static
>
>> + int v = 0;
>> + if(!line) return 0;
>> + char* str = strstr(line, VOL_STR);
>> + char* vol_start = str;
>> + if(!str) return 0;
>> + while (*str && !ParseRules::is_digit(*str))
>> + str++;
>> + v = ink_atoi(str);
>> +
>> + while (*str && ParseRules::is_digit(*str))
>> + str++;
>> + while(*str) {
>> + *vol_start = *str;
>> + vol_start++;
>> + str++;
>> + }
>> + *vol_start = 0;
>> + Debug("cache_init", "returning %d and '%s'", v, line);
>> +
>> + if(v < 0) return 0;
>> + return v;
>> +}
>> +
>>
>> //
>> // Store
>> @@ -291,6 +316,8 @@ Store::read_config(int fd)
>> if (!*n)
>> continue;
>>
>> + int volume_id = getVolume(n);
>> +
>> // parse
>> Debug("cache_init", "Store::read_config: \"%s\"", n);
>>
>> @@ -310,7 +337,9 @@ Store::read_config(int fd)
>> n[len] = 0;
>> char *pp = Layout::get()->relative(n);
>> ns = NEW(new Span);
>> - Debug("cache_init", "Store::read_config - ns = NEW (new Span);
>> ns->init(\"%s\",%" PRId64 ")", pp, size);
>> + ns->vol_num = volume_id;
>> + Debug("cache_init", "Store::read_config - ns = NEW (new Span);
>> ns->init(\"%s\",%" PRId64 "), ns->vol_num=%d",
>> + pp, size, ns->vol_num);
>> if ((err = ns->init(pp, size))) {
>> char buf[4096];
>> snprintf(buf, sizeof(buf), "could not initialize storage
>> \"%s\" [%s]", pp, err);
>
> --
> Igor Galić
>
> Tel: +43 (0) 664 886 22 883
> Mail: i.galic@brainsware.org
> URL: http://brainsware.org/
> GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE
>
Re: git commit: TS-1728: Assign storage entries to volumes
Posted by Igor Galić <i....@brainsware.org>.
----- Original Message -----
> Updated Branches:
> refs/heads/issues/TS-1728 [created] 1d8179df5
>
>
> TS-1728: Assign storage entries to volumes
>
> Introduces the volume assignment in storage.config, on an optional
> basis. No change is needed in the config if you don't want to use
> this feature.
>
> * Add an optional reference to the volume number in storage.config,
> and the Span object, in Store.h and Store.cc
> * Add the volume reference to DiskVol, CacheProcessor::start_internal
> (in Cache.cc)
> * Change cplist_reconfigure (in Cache.cc) to constrain mapping
> * Fix bytes_used and percent_used per volume stats
>
[snip]
n.b.: This codebase is too big/complex for *me* to review, the comments here
are only things that I noticed while reading the diff on a train ride.
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1d8179df/iocore/cache/Cache.cc
> ----------------------------------------------------------------------
> diff --git a/iocore/cache/Cache.cc b/iocore/cache/Cache.cc
> index fbb97f3..4c7b9e4 100644
> --- a/iocore/cache/Cache.cc
> +++ b/iocore/cache/Cache.cc
> @@ -2456,7 +2508,16 @@ cplist_reconfigure()
> /* go through volume config and grow and create volumes */
>
> config_vol = config_volumes.cp_queue.head;
> + for (; config_vol; config_vol = config_vol->link.next) {
form/consistency question: why not have config_vol = config_volumes.cp_queue.head as first part of the for loop?
> + // if volume is given exclusive disks, fill here and continue
> + volume_number = config_vol->number;
> + if (!config_vol->cachep) {
> + continue;
> + }
> + fillExclusiveDisks(config_vol->cachep);
> + }
>
> + config_vol = config_volumes.cp_queue.head;
> for (; config_vol; config_vol = config_vol->link.next) {
>
> size = config_vol->size;
> @@ -2467,6 +2528,11 @@ cplist_reconfigure()
[snip]
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1d8179df/iocore/cache/Store.cc
> ----------------------------------------------------------------------
> diff --git a/iocore/cache/Store.cc b/iocore/cache/Store.cc
> index 9076dc1..c843cbb 100644
> --- a/iocore/cache/Store.cc
> +++ b/iocore/cache/Store.cc
> @@ -28,6 +28,31 @@
> // Global
> Store theStore;
>
> +#define VOL_STR "volume="
consider making this a C++ constant
> +int getVolume(char* line) {
consider defining this function static.
> + int v = 0;
> + if(!line) return 0;
> + char* str = strstr(line, VOL_STR);
> + char* vol_start = str;
> + if(!str) return 0;
> + while (*str && !ParseRules::is_digit(*str))
> + str++;
> + v = ink_atoi(str);
> +
> + while (*str && ParseRules::is_digit(*str))
> + str++;
> + while(*str) {
> + *vol_start = *str;
> + vol_start++;
> + str++;
> + }
> + *vol_start = 0;
> + Debug("cache_init", "returning %d and '%s'", v, line);
> +
> + if(v < 0) return 0;
> + return v;
> +}
> +
>
> //
> // Store
> @@ -291,6 +316,8 @@ Store::read_config(int fd)
> if (!*n)
> continue;
>
> + int volume_id = getVolume(n);
> +
> // parse
> Debug("cache_init", "Store::read_config: \"%s\"", n);
>
> @@ -310,7 +337,9 @@ Store::read_config(int fd)
> n[len] = 0;
> char *pp = Layout::get()->relative(n);
> ns = NEW(new Span);
> - Debug("cache_init", "Store::read_config - ns = NEW (new Span);
> ns->init(\"%s\",%" PRId64 ")", pp, size);
> + ns->vol_num = volume_id;
> + Debug("cache_init", "Store::read_config - ns = NEW (new Span);
> ns->init(\"%s\",%" PRId64 "), ns->vol_num=%d",
> + pp, size, ns->vol_num);
> if ((err = ns->init(pp, size))) {
> char buf[4096];
> snprintf(buf, sizeof(buf), "could not initialize storage
> \"%s\" [%s]", pp, err);
>
>
--
Igor Galić
Tel: +43 (0) 664 886 22 883
Mail: i.galic@brainsware.org
URL: http://brainsware.org/
GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE