Skip to content

Commit 89127e7

Browse files
committed
Add pfc.diskusage files parameters to Pelican
The Lotman purge plugin relies on these parameters to function correctly, so they need to be made configurable. Most of the churn in this commit relates to validating/parsing the watermark values, which may be passed as an integer-encoded percentage or an absolute byte count.
1 parent ad7ca8a commit 89127e7

File tree

11 files changed

+385
-175
lines changed

11 files changed

+385
-175
lines changed

config/config.go

Lines changed: 56 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ import (
3333
"os"
3434
"path/filepath"
3535
"reflect"
36-
"slices"
3736
"sort"
3837
"strconv"
3938
"strings"
@@ -55,6 +54,7 @@ import (
5554
"github.com/pelicanplatform/pelican/param"
5655
"github.com/pelicanplatform/pelican/pelican_url"
5756
"github.com/pelicanplatform/pelican/server_structs"
57+
"github.com/pelicanplatform/pelican/utils"
5858
)
5959

6060
// Structs holding the OAuth2 state (and any other OSDF config needed)
@@ -165,7 +165,6 @@ var (
165165

166166
RestartFlag = make(chan any) // A channel flag to restart the server instance that launcher listens to (including cache)
167167

168-
watermarkUnits = []byte{'k', 'm', 'g', 't'}
169168
validPrefixes = map[ConfigPrefix]bool{
170169
PelicanPrefix: true,
171170
OsdfPrefix: true,
@@ -627,45 +626,6 @@ func handleDeprecatedConfig() {
627626
}
628627
}
629628

630-
func checkWatermark(wmStr string) (bool, int64, error) {
631-
wmNum, err := strconv.Atoi(wmStr)
632-
if err == nil {
633-
if wmNum > 100 || wmNum < 0 {
634-
return false, 0, errors.Errorf("watermark value %s must be a integer number in range [0, 100]. Refer to parameter page for details: https://docs.pelicanplatform.org/parameters#Cache-HighWatermark", wmStr)
635-
}
636-
return true, int64(wmNum), nil
637-
// Not an integer number, check if it's in form of <int>k|m|g|t
638-
} else {
639-
if len(wmStr) < 1 {
640-
return false, 0, errors.Errorf("watermark value %s is empty.", wmStr)
641-
}
642-
unit := wmStr[len(wmStr)-1]
643-
if slices.Contains(watermarkUnits, unit) {
644-
byteNum, err := strconv.Atoi(wmStr[:len(wmStr)-1])
645-
// Bytes portion is not an integer
646-
if err != nil {
647-
return false, 0, errors.Errorf("watermark value %s is neither a percentage integer (e.g. 95) or a valid bytes. Refer to parameter page for details: https://docs.pelicanplatform.org/parameters#Cache-HighWatermark", wmStr)
648-
} else {
649-
switch unit {
650-
case 'k':
651-
return true, int64(byteNum) * 1024, nil
652-
case 'm':
653-
return true, int64(byteNum) * 1024 * 1024, nil
654-
case 'g':
655-
return true, int64(byteNum) * 1024 * 1024 * 1024, nil
656-
case 't':
657-
return true, int64(byteNum) * 1024 * 1024 * 1024 * 1024, nil
658-
default:
659-
return false, 0, errors.Errorf("watermark value %s is neither a percentage integer (e.g. 95) or a valid byte. Bytes representation is missing unit (k|m|g|t). Refer to parameter page for details: https://docs.pelicanplatform.org/parameters#Cache-HighWatermark", wmStr)
660-
}
661-
}
662-
} else {
663-
// Doesn't contain k|m|g|t suffix
664-
return false, 0, errors.Errorf("watermark value %s is neither a percentage integer (e.g. 95) or a valid byte. Bytes representation is missing unit (k|m|g|t). Refer to parameter page for details: https://docs.pelicanplatform.org/parameters#Cache-HighWatermark", wmStr)
665-
}
666-
}
667-
}
668-
669629
func setupTranslation() error {
670630
err := en_translations.RegisterDefaultTranslations(validate, GetEnTranslator())
671631
if err != nil {
@@ -1326,18 +1286,63 @@ func InitServer(ctx context.Context, currentServers server_structs.ServerType) e
13261286
}
13271287

13281288
if param.Cache_LowWatermark.IsSet() || param.Cache_HighWaterMark.IsSet() {
1329-
lowWmStr := param.Cache_LowWatermark.GetString()
1330-
highWmStr := param.Cache_HighWaterMark.GetString()
1331-
ok, highWmNum, err := checkWatermark(highWmStr)
1332-
if !ok && err != nil {
1333-
return errors.Wrap(err, "invalid Cache.HighWaterMark value")
1289+
lowWm, lwmIsAbs, err := utils.ValidateWatermark(param.Cache_LowWatermark.GetName(), false)
1290+
if err != nil {
1291+
return err
13341292
}
1335-
ok, lowWmNum, err := checkWatermark(lowWmStr)
1336-
if !ok && err != nil {
1337-
return errors.Wrap(err, "invalid Cache.LowWatermark value")
1293+
highWm, hwmIsAbs, err := utils.ValidateWatermark(param.Cache_HighWaterMark.GetName(), false)
1294+
if err != nil {
1295+
return err
1296+
}
1297+
if lwmIsAbs == hwmIsAbs && lowWm >= highWm {
1298+
// Use config strings in error to present the configured values (as opposed to whatever conversion we get from validation)
1299+
return errors.Errorf("invalid Cache.HighWaterMark/LowWaterMark values. The high watermark must be greater than the low "+
1300+
"watermark. Got %s (low) and %s (high)", param.Cache_LowWatermark.GetString(), param.Cache_HighWaterMark.GetString())
1301+
}
1302+
}
1303+
1304+
if param.Cache_FilesBaseSize.IsSet() || param.Cache_FilesNominalSize.IsSet() || param.Cache_FilesMaxSize.IsSet() {
1305+
// Must have high/low watermarks
1306+
if !param.Cache_LowWatermark.IsSet() || !param.Cache_HighWaterMark.IsSet() {
1307+
return errors.New("If any of Cache parameters 'FilesBaseSize', 'FilesNominalSize', or 'FilesMaxSize' is set, then Cache.LowWatermark " +
1308+
"and Cache.HighWatermark must also be set")
1309+
}
1310+
1311+
// Further, if one is set, all three must be set
1312+
if !param.Cache_FilesBaseSize.IsSet() || !param.Cache_FilesNominalSize.IsSet() || !param.Cache_FilesMaxSize.IsSet() {
1313+
return errors.New("If any of Cache parameters 'FilesBaseSize', 'FilesNominalSize', or 'FilesMaxSize' is set, all three must be set")
13381314
}
1339-
if lowWmNum >= highWmNum {
1340-
return fmt.Errorf("invalid Cache.HighWaterMark and Cache.LowWatermark values. Cache.HighWaterMark must be greater than Cache.LowWaterMark. Got %s, %s", highWmStr, lowWmStr)
1315+
1316+
var base, nominal, max float64
1317+
var err error
1318+
// Watermark validation will error if these parameters are not absolute, so we can ignore that output
1319+
if base, _, err = utils.ValidateWatermark(param.Cache_FilesBaseSize.GetName(), true); err != nil {
1320+
return err
1321+
}
1322+
if nominal, _, err = utils.ValidateWatermark(param.Cache_FilesNominalSize.GetName(), true); err != nil {
1323+
return err
1324+
}
1325+
if max, _, err = utils.ValidateWatermark(param.Cache_FilesMaxSize.GetName(), true); err != nil {
1326+
return err
1327+
}
1328+
if base >= nominal || nominal >= max {
1329+
return errors.Errorf("invalid Cache.FilesBaseSize/FilesNominalSize/FilesMaxSize values. The base size must be less than the "+
1330+
"nominal size, and the nominal size must be less than the max size. Got %s (base), %s (nominal), and %s (max)",
1331+
param.Cache_FilesBaseSize.GetString(), param.Cache_FilesNominalSize.GetString(), param.Cache_FilesMaxSize.GetString())
1332+
}
1333+
1334+
// File sizes must also be less than the low watermark, but that's not straightforward to check, especially when the cache spread across
1335+
// multiple disks and the low watermark is configured as a relative value. If such bad config is passed, xrootd will fail to startup with
1336+
// a message about what to do, and as much as I'd like to handle that early, I'll leave it to xrootd for now.
1337+
}
1338+
if param.Cache_EnableLotman.GetBool() {
1339+
// pfc.diskusage file directives _must_ be set.
1340+
// We already validate that one means all three, but I'm duplicating that here to make this safer in the case we switch orders
1341+
// in the future.
1342+
if !param.Cache_FilesBaseSize.IsSet() || !param.Cache_FilesNominalSize.IsSet() || !param.Cache_FilesMaxSize.IsSet() ||
1343+
!param.Cache_LowWatermark.IsSet() || !param.Cache_HighWaterMark.IsSet() {
1344+
return errors.New("If Cache.EnableLotman is true, the following Cache parameters must also be set: HighWaterMark, LowWaterMark, " +
1345+
"FilesBaseSize, FilesNominalSize, FilesMaxSize")
13411346
}
13421347
}
13431348

config/config_test.go

Lines changed: 0 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -549,117 +549,6 @@ func TestSetPreferredPrefix(t *testing.T) {
549549
})
550550
}
551551

552-
func TestCheckWatermark(t *testing.T) {
553-
t.Parallel()
554-
555-
t.Run("empty-value", func(t *testing.T) {
556-
ok, num, err := checkWatermark("")
557-
assert.False(t, ok)
558-
assert.Equal(t, int64(0), num)
559-
assert.Error(t, err)
560-
})
561-
562-
t.Run("string-value", func(t *testing.T) {
563-
ok, num, err := checkWatermark("random")
564-
assert.False(t, ok)
565-
assert.Equal(t, int64(0), num)
566-
assert.Error(t, err)
567-
})
568-
569-
t.Run("integer-greater-than-100", func(t *testing.T) {
570-
ok, num, err := checkWatermark("101")
571-
assert.False(t, ok)
572-
assert.Equal(t, int64(0), num)
573-
assert.Error(t, err)
574-
})
575-
576-
t.Run("integer-less-than-0", func(t *testing.T) {
577-
ok, num, err := checkWatermark("-1")
578-
assert.False(t, ok)
579-
assert.Equal(t, int64(0), num)
580-
assert.Error(t, err)
581-
})
582-
583-
t.Run("decimal-fraction-value", func(t *testing.T) {
584-
ok, num, err := checkWatermark("0.55")
585-
assert.False(t, ok)
586-
assert.Equal(t, int64(0), num)
587-
assert.Error(t, err)
588-
})
589-
590-
t.Run("decimal-int-value", func(t *testing.T) {
591-
ok, num, err := checkWatermark("15.55")
592-
assert.False(t, ok)
593-
assert.Equal(t, int64(0), num)
594-
assert.Error(t, err)
595-
})
596-
597-
t.Run("int-value", func(t *testing.T) {
598-
ok, num, err := checkWatermark("55")
599-
assert.True(t, ok)
600-
assert.Equal(t, int64(55), num)
601-
assert.NoError(t, err)
602-
})
603-
604-
t.Run("byte-value-no-unit", func(t *testing.T) {
605-
ok, num, err := checkWatermark("105")
606-
assert.False(t, ok)
607-
assert.Equal(t, int64(0), num)
608-
assert.Error(t, err)
609-
})
610-
611-
t.Run("byte-value-no-value", func(t *testing.T) {
612-
ok, num, err := checkWatermark("k")
613-
assert.False(t, ok)
614-
assert.Equal(t, int64(0), num)
615-
assert.Error(t, err)
616-
})
617-
618-
t.Run("byte-value-wrong-unit", func(t *testing.T) {
619-
ok, num, err := checkWatermark("100K") // Only lower case is accepted
620-
assert.False(t, ok)
621-
assert.Equal(t, int64(0), num)
622-
assert.Error(t, err)
623-
624-
ok, num, err = checkWatermark("100p")
625-
assert.False(t, ok)
626-
assert.Equal(t, int64(0), num)
627-
assert.Error(t, err)
628-
629-
ok, num, err = checkWatermark("100byte")
630-
assert.False(t, ok)
631-
assert.Equal(t, int64(0), num)
632-
assert.Error(t, err)
633-
634-
ok, num, err = checkWatermark("100bits")
635-
assert.False(t, ok)
636-
assert.Equal(t, int64(0), num)
637-
assert.Error(t, err)
638-
})
639-
640-
t.Run("byte-value-correct-unit", func(t *testing.T) {
641-
ok, num, err := checkWatermark("1000k")
642-
assert.True(t, ok)
643-
assert.Equal(t, int64(1000*1024), num)
644-
assert.NoError(t, err)
645-
646-
ok, num, err = checkWatermark("1000m")
647-
assert.True(t, ok)
648-
assert.Equal(t, int64(1000*1024*1024), num)
649-
assert.NoError(t, err)
650-
651-
ok, num, err = checkWatermark("1000g")
652-
assert.True(t, ok)
653-
assert.Equal(t, int64(1000*1024*1024*1024), num)
654-
assert.NoError(t, err)
655-
656-
ok, num, err = checkWatermark("1000t")
657-
assert.True(t, ok)
658-
assert.Equal(t, int64(1000*1024*1024*1024*1024), num)
659-
assert.NoError(t, err)
660-
})
661-
}
662-
663552
func TestInitServerUrl(t *testing.T) {
664553
ctx := testConfigContext(t)
665554

docs/parameters.yaml

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1284,26 +1284,83 @@ components: ["cache"]
12841284
---
12851285
name: Cache.LowWatermark
12861286
description: |+
1287-
A value of cache disk usage that stops the purging of cached files.
1287+
Whenever the cache initiates file purging, it will attempt to clean files until its cumulative disk
1288+
usages reaches this value. Note that "cache disk usage" is calculated based on the cache's entire set of
1289+
configured disks, not just data directories from those disks.
12881290
12891291
The value should be either a percentage integer of total available disk space (default is 90),
12901292
or a number suffixed by k, m, g, or t. In which case, they must be absolute sizes in k (kilo-),
12911293
m (mega-), g (giga-), or t (tera-) bytes, respectively.
1294+
1295+
For more information, see the [xrootd pfc documentation](https://xrootd.web.cern.ch/doc/dev56/pss_config.pdf) for
1296+
`pfc.diskusage`.
12921297
type: string
12931298
default: 90
12941299
components: ["cache"]
12951300
---
12961301
name: Cache.HighWaterMark
12971302
description: |+
1298-
A value of cache disk usage that triggers the purging of cached files.
1303+
When the cache's disk usage exceeds this value, file purging is triggered. Note that "cache disk usage"
1304+
is calculated based on the cache's entire set of configured disks, not just data directories from those disks.
12991305
13001306
The value should be either a percentage integer of total available disk space (default is 95),
13011307
or a number suffixed by k, m, g, or t. In which case, they must be absolute sizes in k (kilo-),
13021308
m (mega-), g (giga-), or t (tera-) bytes, respectively.
1309+
1310+
For more information, see the [xrootd pfc documentation](https://xrootd.web.cern.ch/doc/dev56/pss_config.pdf) for
1311+
`pfc.diskusage`.
13031312
type: string
13041313
default: 95
13051314
components: ["cache"]
13061315
---
1316+
name: Cache.FilesMaxSize
1317+
description: |+
1318+
A value that sets the maximum cumulative size of files that can be stored in the cache's data directories (specified
1319+
by `Cache.StorageLocation` and `Cache.DataLocations). When either this value or `Cache.HighWaterMark` is exceeded, the
1320+
cache will begin purging files until it reaches the `Cache.FilesNominal` value. If the cache's disk usage is still in
1321+
excess of the `Cache.LowWaterMark`, the cache will continue purging files until it reaches the `Cache.FilesBase` value.
1322+
1323+
Unlike watermark values, this value _must_ be suffixed by a unit of k, m, g, or t, which represent kilobytes, megabytes,
1324+
gigabytes, and terabytes, respectively. All `Cache.Files*Size` parameters must be less than the cache's calculated low
1325+
watermark, which may be configured as a percentage of total disk space from multiple disks.
1326+
1327+
For more information, see the [xrootd pfc documentation](https://xrootd.web.cern.ch/doc/dev56/pss_config.pdf) for
1328+
`pfc.diskusage`.
1329+
type: string
1330+
default: none
1331+
components: ["cache"]
1332+
---
1333+
name: Cache.FilesNominalSize
1334+
description: |+
1335+
A value that sets the "nominal" cumulative size of files that can be stored in the cache's data directory. When files
1336+
in the cache exceed the `Cache.FilesMax` value, or if the cache's overall disk exceeds its `Cache.HighWaterMark` value,
1337+
the cache will begin purging files until it reaches this value. If the cache's disk usage is still in excess of the
1338+
`Cache.LowWaterMark`, the cache will continue purging files until it reaches the `Cache.FilesBase`.
1339+
1340+
Unlike watermark values, this value _must_ be suffixed by a unit of k, m, g, or t, which represent kilobytes, megabytes,
1341+
gigabytes, and terabytes, respectively. All `Cache.Files*Size` parameters must be less than the cache's calculated low
1342+
watermark, which may be configured as a percentage of total disk space from multiple disks.
1343+
1344+
For more information, see the [xrootd pfc documentation](https://xrootd.web.cern.ch/doc/dev56/pss_config.pdf) for
1345+
`pfc.diskusage`.
1346+
type: string
1347+
default: none
1348+
components: ["cache"]
1349+
---
1350+
name: Cache.FilesBaseSize
1351+
description: |+
1352+
A value that sets the "base" cumulative size of files that can be stored in the cache's data directory. This is the
1353+
stopping point for the cache's purging routines.
1354+
1355+
Unlike watermark values, this value _must_ be suffixed by a unit of k, m, g, or t, which represent kilobytes, megabytes,
1356+
gigabytes, and terabytes, respectively.All `Cache.Files*Size` parameters must be less than the cache's calculated low
1357+
watermark, which may be configured as a percentage of total disk space from multiple disks.
1358+
1359+
For more information, see the [xrootd pfc documentation](https://xrootd.web.cern.ch/doc/dev56/pss_config.pdf) for `pfc.diskusage`.
1360+
type: string
1361+
default: none
1362+
components: ["cache"]
1363+
---
13071364
name: Cache.EnableVoms
13081365
description: |+
13091366
Enable X.509 / VOMS-based authentication for the cache. This allows HTTP clients

lotman/lotman_linux.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -606,11 +606,11 @@ func convertWatermarkToBytes(value string, totalDiskSpace uint64) (uint64, error
606606
if len(value) > 1 {
607607
suffix := strings.ToLower(string(value[len(value)-1]))
608608
if multiplier, exists := suffixMultipliers[suffix]; exists {
609-
number, err := strconv.ParseUint(value[:len(value)-1], 10, 64)
609+
number, err := strconv.ParseFloat(value[:len(value)-1], 64)
610610
if err != nil {
611611
return 0, err
612612
}
613-
return number * multiplier, nil
613+
return uint64(number * float64(multiplier)), nil
614614
}
615615
}
616616

@@ -622,7 +622,6 @@ func convertWatermarkToBytes(value string, totalDiskSpace uint64) (uint64, error
622622
return uint64((percentage / 100) * float64(totalDiskSpace)), nil
623623
}
624624

625-
626625
// Divide the remaining space among lots' dedicatedGB values -- we don't ever want to
627626
// dedicate more space than we have available, as indicated by the HWM of the cache. This is because
628627
// our model is that as long as a lot stays under its dedicated GB, its data is safe in the cache --

lotman/lotman_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -743,8 +743,8 @@ func TestConvertWatermarkToBytes(t *testing.T) {
743743
testCases := []testCase{
744744
{
745745
Name: "Valid 'k' suffix",
746-
Watermark: "100k",
747-
Expected: uint64(100000), // 100KB
746+
Watermark: "100.1k",
747+
Expected: uint64(100100), // 100KB
748748
ErrorExpected: false,
749749
},
750750
{
@@ -767,8 +767,8 @@ func TestConvertWatermarkToBytes(t *testing.T) {
767767
},
768768
{
769769
Name: "No suffix is percentage",
770-
Watermark: "50",
771-
Expected: uint64(500000000000), // 500GB
770+
Watermark: "50.5",
771+
Expected: uint64(505000000000), // 500GB
772772
ErrorExpected: false,
773773
},
774774
{

0 commit comments

Comments
 (0)