Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

afs-client-accessd: allow configuration of export time #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

afs-client-accessd: allow configuration of export time #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 48 additions & 4 deletions admin/afs-client-accessd/afs-client-accessd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,15 @@ Example:

AUDIT_PATH => '/usr/afs/logs/FileAudit',

=item EXPORT_TIME
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this name might be a bit confusing, since it is not set at the "export time" but the hour of the day (in utc) the export is scheduled. Internally you've converted the value to be the epoch time by multiply the value the user gave by the number of seconds per day.

I think it would be more clear to all this EXPORT_HOUR since it is the hour the export is scheduled.


This specifies the hour of the day (UTC) in which the *.sqlite files should be
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace "should be" with "are scheduled". Should mention this is the "approximate scheduled time of day".

exported to the central host. The default is 0 (midnight).

Example:

EXPORT_TIME => 16,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Export hour?


=item SQLITE_PREFIX

This specifies the directory in which the local disk databases will be stored
Expand Down Expand Up @@ -581,6 +590,7 @@ read_config()

MSGRCV_WORKAROUND => 0,
USE_FQDN => 0,
EXPORT_TIME => 0,
);

if (!-r $CONFIG_FILE) {
Expand All @@ -601,6 +611,11 @@ read_config()
$newcfg{$_} = $override{$_};
}

if ($newcfg{EXPORT_TIME} < 0 || $newcfg{EXPORT_TIME} > 24) {
$newcfg{EXPORT_TIME} = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if possible, but it would be better if we could log a warning if changing a value which is out of range.

}
$newcfg{EXPORT_TIME} = $newcfg{EXPORT_TIME} * 60 * 60;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not convert the new config value here. If you want to do the conversion once, maybe create a new variable? If feels very side-effectly.


my $err = '';
if (! -d $newcfg{SQLITE_PREFIX}) {
$err .= "SQLITE_PREFIX dir $newcfg{SQLITE_PREFIX} does not exist\n";
Expand Down Expand Up @@ -1230,6 +1245,27 @@ spawn_exporter()
exit(0);
}

sub
time_to_export()
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After thinking about this a bit, I think this function is not clear to me because it is doing several different things. I feel it would be more clear if we had functions which had more clear intent.

For example, for this feature we need to do the following, which can be though of as functions. Each take arguments which evaluate to one (and only one) result value.

  1. Determine the next scheduled time as a unix epoch time from the "hours" specified by the user.
    1b. To find 1, we need the value of today's midnight, as a unix epoch time.
  2. Determine if a given time is within a window of the current time (now).

In addition, we can make this more clear by declaring constant values as perl constants. That helps because it shows intent to the programmer and the interpreter which things are not going to change as the program runs.

Here's some example code (not tested) that shows what I mean.

use const SEC_PER_HOUR => 60 * 60;
use const SEC_PER_DAY => 24 * 60 * 60;
use const FLUCTUATION => 90;

# Return the previous midnight time as a unix epoch time.
sub
midnight_time()
{
   my $now = time(); 
   return $now - ($now % SEC_PER_DAY);
}

# Return the next scheduled time as a unix epoch time.
sub
schedule_time()
{
   my $hour = shift;
   return midnight_time() + ($hour * SEC_PER_HOUR);
}

# Return true if target time is within the time window.
sub
in_window()
{
    my $target = shift;
    my $now = time()

    my $range = (ALARM_INTERVAL / 2) + FLUCTUATION;
    my $lower = $now - $range;
    my $upper = $now + $range;

    return $lower < $target && $target <= $upper;
}

....

    if (...) {
       if (in_window(schedule_time($CFG{EXPORT_HOUR}))) {
                                                                                                                                        2,5           Top

my $current_time = time();

my $sec_per_day = 24 * 60 * 60;
my $midnight = $current_time - ($current_time % $sec_per_day);

my $exp_time = $midnight + $CFG{EXPORT_TIME};

my $fluctuation = 90;
my $range = (ALARM_INTERVAL / 2) + $fluctuation;

if (($current_time > ($exp_time - $range)) &&
($current_time <= ($exp_time + $range))) {
return 1;
} else {
return 0;
}
}

my $checksignals_firstrun = 1;
# check for changes flagged by a signal handler
sub
Expand Down Expand Up @@ -1295,10 +1331,6 @@ checksignals()

stop_collection($next_dbname);

# export database in the background
d("Spawning exporter due to db name change");
spawn_exporter();

$CUR_DBNAME = $next_dbname;

# set up our sqlite stuff again so we can record new data to it
Expand All @@ -1322,6 +1354,18 @@ checksignals()
spawn_exporter();
}

my $sig_alarm = 0;

if (!$SIGNAL_SHUTDOWN && !$SIGNAL_CHILD && !$SIGNAL_CONFIG) {
$sig_alarm = 1;
}

if (!$EXPORTER_PID && $sig_alarm && time_to_export()) {
# export database in the background
d("Spawning exporter due to time_to_export");
spawn_exporter();
}

$checksignals_firstrun = 0;

$SIGNAL_SHUTDOWN = 0;
Expand Down
6 changes: 6 additions & 0 deletions admin/afs-client-accessd/afs-client-accessd.conf.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ AUDIT_PATH => '/usr/afs/logs/FileAudit',

SQLITE_PREFIX => '/var/afs-accessdb',

# EXPORT_TIME specifies the hour of the day in which the databases from
# SQLITE_PREFIX should be exported to the central host. The timezone is UTC and
# the default value is 0 (midnight).

EXPORT_TIME => 16,

# RHEL 5.x broken msgrcv workaround. msgrcv fails supriously
# on older versions of RHEL 5.x without setting the errno. strace
# shows ERESTARTNOHAND errors.
Expand Down