Currently, we inhibit various -Wstrict-prototypes warnings via
"#pragma GCC diagnostic warning". Some older compilers (like gcc 4.1
on RHEL5) don't understand the pragma, but still need the warning
inhibited in order to build with --enable-checking. So just inhibit
this warning via command-line CFLAGS instead of using #pragma
directives.
Change-Id: If5b37cba460bc243dbaeadc6305908aed0ad34d4
Reviewed-on: https://gerrit.openafs.org/15219
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Since the original IBM code import, rx_NewConnection allocates a new
rx_connection on each call by invoking rxi_AllocConnection -> rxi_Alloc.
rxi_Alloc will panic if no memory is available; therefore
rxi_AllocConnection and rx_NewConnection can never return NULL.
Remove the superfluous checks of the return from rx_NewConnection.
Add equivalent asserts to guard against future breakage.
While here, reorder operations in ubeacon_ReInitServer so we can
eliminate some temporary variables.
Change-Id: I6d00195277d689d80cc6cd7963d8b0a5e5630229
Reviewed-on: https://gerrit.openafs.org/14607
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Ubik invariants (see src/ubik/ubik.p.h) document that for proper quorum
elections, the VOTE service requires:
"#SMALLTIME and #BIGTIME must be larger than
#RPCTIMEOUT+max(#RPCTIMEOUT, #POLLTIME)"
Solving this for VOTE connection Rx dead timeout yields:
Rx dead timeout < 30s
However, setting rx dead time only protects the election if a db host
goes down. If a db server is still alive (responding with acks and
pings) but the VOTE_Beacon reply is delayed for some reason, the entire
election can still take too long and cause a loss of quorum.
Specify a hard dead time in ubeacon_NewVOTEConnection. This ensures
that all VOTE RPCs for all ubik servers (and thus, all elections) will
end within the hard dead time,
Change-Id: I90d0cd2ec9ecd001d448068960592067e6d8dc77
Reviewed-on: https://gerrit.openafs.org/15553
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Ubik invariants (see src/ubik/ubik.p.h) document that for proper quorum
elections, the VOTE service requires:
"#SMALLTIME and #BIGTIME must be larger than
#RPCTIMEOUT+max(#RPCTIMEOUT, #POLLTIME)"
Solving this for VOTE connection Rx dead timeout yields:
Rx dead timeout < 30s
The current default rx_connDeadTime is 12s; if this value is in force
when VOTE client connections are created or recreated, ubik elections
will work correctly.
(Apparently, RPCTIMEOUT 20s was once intended to be used for this value,
but this constant has not been referenced by any live code since the
original IBM code import.)
However, since the original IBM code import, vlserver has set
rx_connDeadTime to 50s. In 1.6.x and older releases, this was only done
after the ubik VOTE and DISK client connections had been established;
therefore the initial VOTE connections had the correct default
rx_connDeadTime of 12s. However, if a VOTE client connection ever failed
(e.g., due to a down vlserver), it would be reestablished with the 50s
timeout. Because this violates a ubik invariant, it may lead to random
quorum disturbances if further vlserver outages occur. This is because
the election results may be delayed for up to 50s until the
multi_VOTE_Beacon to the down vlserver times out. If the vlserver goes
down just a few seconds before the next election, this delay is just
long enough to cause the syncsite's votes to expire (SMALLTIME 60s),
triggering a temporary loss of quorum even when there are sufficient yes
votes to maintain quorum.
As of commit 3e531db9ce (vlserver: rx_SetRxDeadTime before ubik init)
introduced with OpenAFS 1.8.0, rx_connDeadTime is set to 50s for ALL
vlserver connections, including the initial ubik VOTE and DISK client
connections. However, as explained above, this violates a ubik
invariant for the VOTE service. This results in a consistent loss of
quorum every time a vlserver goes down within a few seconds of a new
election, even if there are sufficient votes to maintain quorum.
Create and use a new function ubeacon_NewVOTEConnection to create all
VOTE client connections with an rx dead time VOTE_RPCTIMEOUT which
complies with the ubik invariants. This allows ubik applications like
vlserver to change rx_connDeadTime without affecting ubik election
timing.
Note: vlserver is the only in-tree ubik server affected by this issue.
Even though buserver (src/budb/server.c) also sets a problematic
rx_connDeadTime of 60s at initialization, this is immediately reset to
12s by a subsequent call to rx_InitHost. (This buserver anomaly will be
addressed in another commit.) The other ubik servers (ptserver and
kaserver) always use the default 12s time.
Change-Id: I63e2c46b6097c9c2c89e6e858e3958846c6cb190
Reviewed-on: https://gerrit.openafs.org/14608
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
clang-16 is flagging unused-but-set variables which result in build
errors when --enable-warning is turned on.
remote.c:485:15: error: variable 'pass' set but not used
[-Werror,-Wunused-but-set-variable]
afs_int32 pass;
^
These variables are actually used in specific cases depending on
build configuration (e.g. AFS_PTHREAD_ENV, etc.).
Relocate these variables so they are fully defined or referenced within
preprocessor '#if' blocks.
Change-Id: I900a192ec781be4ab4453769e1d8f0cc48fe5757
Reviewed-on: https://gerrit.openafs.org/15177
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
When running a dbserver without any remote sites, we become the sync
site immediately, since there aren't any other sites so we don't need
to do any processing of votes. The ubik database can thus be accessed
immediately, except we cannot start ubik write transactions right
away, since UBIK_RECHAVEDB will not be set, causing
urecovery_AllBetter() to fail.
UBIK_RECHAVEDB is not set immediately, because urecovery_Interact
sleeps 4 seconds at the beginning of its loop, so no recovery flags
will be set for the first 4 seconds during startup. This makes it
impossible to start any ubik write transactions for the first 4
seconds, even if we're the only site. That may not be a significant
amount of time for a user (since a dbserver daemon doesn't startup too
frequently), but this can cause huge delays for automated testing of
dbservers.
To get rid of this unnecessary delay, just skip this delay the first
time we go through the urecovery_Interact loop.
Change-Id: Ie6653b7b742dcf37798a6bf340b29c283ac3bc4c
Reviewed-on: https://gerrit.openafs.org/14803
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The ubeacon_updateUbikNetworkAddress() prototype does not match the
function definition. The ubik_host parameter is declared as an unbounded
array in the prototype but is defined as a bounded array. As of GCC 12,
a warning is issued for the mismatch:
error: argument 1 of type ‘afs_uint32[256]’ {aka ‘unsigned int[256]’}
with mismatched bound [-Werror=array-parameter=]
ubeacon_updateUbikNetworkAddress(
afs_uint32 ubik_host[UBIK_MAX_INTERFACE_ADDR])
note: previously declared as ‘afs_uint32[]’ {aka ‘unsigned int[]’}
extern int ubeacon_updateUbikNetworkAddress(afs_uint32 ubik_host[]);
Restore the ubik_host array length in the function prototype, which was
dropped in commit 9020e6e2f0 (ubik: Defer
updateUbikNetworkAddress until after RX startup).
Change-Id: I8189effc5b68ef8c1b45b4107f5e22e44ecf59fd
Reviewed-on: https://gerrit.openafs.org/14767
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Clang-13 changed the default for the unused-but-set-variable resulting
in build warnings/errors with the following type of messages
vsprocs.c:3493:25: error: variable 'tentries' set but not used
[-Werror,-Wunused-but-set-variable]
afs_int32 nentries, tentries = 0;
The locations where these local variables are being flagged show that
while the variables are being updated, they are actually never used for
anything (e.g. used as part of an assignment to another variable, passed
as a parameter, used for as returned value, etc.)
Remove the variables being flagged by the clang-13 compiler.
Removal of these variables will not alter the overall functionality of
the code.
Change-Id: I521ea1323837e0d293e6f45eb8c15d5e05765f19
Reviewed-on: https://gerrit.openafs.org/14775
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
When using our multi_Rx mechanism, callers can use either multi_End or
multi_End_Ignore at the end of the multi_Rx block. Among other things,
these macros run 'rx_EndCall(call, code)' for each call in the
multi_Rx invocation that hasn't already ended. For multi_End, 'code'
is RX_USER_ABORT, and for multi_End_Ignore, 'code' is 0; the macros
are otherwise equivalent.
When multi_End is used, this means any un-ended calls are aborted with
RX_USER_ABORT, and the call immediately ends. But when
multi_End_Ignore is used, the call is not aborted, and so we must wait
for the peer to acknowledge that it has received our packets before
ending (done via an rxi_ReadProc call in rx_EndCall).
This means that if a caller multi_Abort's out of a multi call and uses
multi_End_Ignore, we'll wait for the peer to acknowledge our packets
for all of the calls we haven't processed. Waiting for that is a
complete waste of time, since we don't care about the results of those
calls (since we multi_Abort'd). This doesn't matter much if those
calls are responded to promptly, but if the peer is not up or is just
slow, it can cause us to wait several seconds until we timeout.
There are currently only three users of multi_End_Ignore:
- DoProbe in src/ubik/recovery.c
- MultiBreakCallBackAlternateAddress_r in src/viced/callback.c
- MultiProbeAlternateAddress_r in src/viced/callback.c
All of these use multi_Rx to try and probe multiple IPs for the same
machine in parallel, and so some of the calls may very well be trying
to contact unreachable IPs; we only need one to work for the call to
succeed.
To avoid the unnecessary delays in these codepaths, convert them to
use multi_End. Change multi_End_Ignore to be the same as multi_End,
and multi_Finalize_Ignore to the same as multi_Finalize, to make sure
the bad behavior is not used. The _Ignore macros/functions are now
unused in the tree, but keep them around for now since
multi_Finalize_Ignore is exported by libafsrpc.
Thanks to mbarbosa@sinenomine.net for discovering this weird behavior.
Change-Id: I65536a0975bd7a16bb805555943c032c5e6afdf3
Reviewed-on: https://gerrit.openafs.org/14595
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Numerous types and constants are defined in our various RPC-L files
that are never used or referenced by anything. Remove them.
Change-Id: I0b03be1ce0e186a88f80d2f3f7a66a1e25965ff3
Reviewed-on: https://gerrit.openafs.org/14404
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
To make the ubik_Call* functions cleaner, consolidate code that finds
the index of the connection associated with a host into a new function.
No functional change should be incurred by this commit.
Change-Id: I320d7a41221cb533e8d077c412f872152ac43b75
Reviewed-on: https://gerrit.openafs.org/14060
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The RPC-L type sampleName and related constant UMAXNAMELEN are not
referenced by anything, and have been unused since OpenAFS 1.0. Remove
the unused definitions.
Change-Id: I21a11d9db9ed80547de8685623fb09f9a86934f1
Reviewed-on: https://gerrit.openafs.org/14386
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
In urecovery_Interact, if any of our operations fail around
calling DISK_GetFile, we will jump to FetchEndCall and eventually
unlink 'pbuffer'. But if we failed before opening our .DB0.TMP file,
the contents of 'pbuffer' will not be initialized yet.
During most iterations of the recovery loop, the contents of 'pbuffer'
will be filled in from previous loops, and it should always stay the
same, so it's not a big problem. But if this is the first iteration of
the loop, the contents of 'pbuffer' may be stack garbage.
Solve this in two ways. To make sure we don't use garbage contents in
'pbuffer', memset the whole thing to zeroes at the beginning of
urecovery_Interact(). And then to make sure we're not reusing
'pbuffer' contents from previous iterations of the loop, also clear
the first character to NUL each time we arrive at this area of the
recovery code. And avoid unlinking anything if pbuffer starts with a
NUL.
Commit 44e80643 (ubik: Avoid unlinking garbage) fixes the same issue,
but only fixed it in the SDISK_SendFile codepath in remote.c.
Change-Id: Ica39e66efa89562068a4be3a14b2d13594b77f6d
Reviewed-on: https://gerrit.openafs.org/14153
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The SVOTE_GetSyncSite RPC was intended to provide the IP address of the
current sync-site. Unfortunately, the RPC-L incorrectly defined ahost as
an input argument instead of an output argument. As a result, the IP
address in question is not returned to the callers of SVOTE_GetSyncSite.
Moreover, calls to this RPC must be made through connections associated
with the VOTE_SERVICE_ID. Sadly, the ubik_Call* functions call
SVOTE_GetSyncSite using connections associated with the USER_SERVICE_ID.
Consequently, the server getting this request returns RXGEN_OPCODE,
meaning that this RPC is not implemented by the service in question.
Since RPC arguments cannot be changed without causing compatibility
issues between different client / server versions and the RPC in
question is being called through the wrong service id, remove
SVOTE_GetSyncSite and its callers. Considering that in all versions of
OpenAFS calls to this RPC always return RXGEN_OPCODE, no behavior
change is introduced by this commit.
Also, remove the "chaseCount logic" from the ubik_Call* functions.
This logic prevents the loop counter from being moved backwards
indefinitely, resulting in an infinite loop. Fortunately, without the
VOTE_GetSyncSite() calls this counter cannot be moved backwards more
than once.
Change-Id: Idd071583e8f67109e003f7a5675de02a235e5809
Reviewed-on: https://gerrit.openafs.org/14043
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Rename ubik_dbase->flags to ubik_dbase->dbFlags, to make it easier to
distinguish between other fields and variables just called 'flags'.
Change-Id: I17258f9a65e989943d066307e332550d66ca7500
Reviewed-on: https://gerrit.openafs.org/13864
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Commit e4ac552a (ubik: Introduce version lock) added UBIK_VERSION_LOCK
and version_data. The commit message mentions that holding either
UBIK_VERSION_LOCK or DBHOLD is enough to be able to read the protected
items and both locks must be held to modify them, but this isn't
mentioned in the actual code.
Add a comment explaining these locking rules, to make these rules
clearer to readers.
Change-Id: I715f89695add6d94e13d6ee1dc6addd1e748d3fd
Reviewed-on: https://gerrit.openafs.org/13863
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Log when urecovery_CheckTid aborts/ends a running remote transaction.
This is usually a rare event, occurring when some ubik sites get
"stuck" or confused about the state of the quorum. Logging some
details when this happens can be useful when investigating issues
post-mortem, or just to see why a transaction failed.
Change-Id: If0a7cd134aaac3722fe7214a1d8f0efab550ad11
Reviewed-on: https://gerrit.openafs.org/13862
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
In OpenAFS 1.0, the way we made dbserver RPC calls was to pass the
relevant RPC and arguments to ubik_Call()/ubik_Call_New(), which
coerced all of the RPC arguments into 'long's. To make this more
typesafe, in commit 4478d3a9 (ubik-call-sucks-20060703) most callers
were converted to use ubik_RPC_name()-style calls, which used
functions autogenerated by rxgen.
This latter approach, however, only lets us use the ubik_Call-style
site selection code with RPCs processed by rxgen; we can't insert
additional code to run before or after the relevant RPC.
To make our dbserver calls more flexible, but avoid coercing all of
our arguments into 'long's again, move back to the ubik_Call()-style
approach, but use actual typed arguments with a callback function and
a rock. Call it ubik_CallRock().
With this commit rxgen still generates the ubik_RPC_name()-style
stubs, but the stubs just call ubik_CallRock with a generated callback
function, instead of spitting out the equivalent of ubik_Call() in the
generated code itself.
To try to ensure that this commit doesn't incur any unintended extra
changes, make ubik_CallRock consist of the generated code that was
inside rxgen before this commit. This is almost identical to
ubik_Call, but not quite; consolidating these two functions can happen
in a future commit if desired.
Change-Id: I0c3936e67a40e311bff32110b2c80696414b52d4
Reviewed-on: https://gerrit.openafs.org/13987
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
We currently specify the signature of the 'secproc' function callback
in multiple places. Consolidate them into a single typedef.
Change-Id: Ic785f47fc726bff6c37f7fd826f1e2626d006776
Reviewed-on: https://gerrit.openafs.org/13986
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Currently, SVOTE_Debug/SVOTE_DebugOld examine some ubik internal state
without any locks, because the speed of these functions is more
important than accuracy.
However, one of the pieces of data we examine is ubik_currentTrans,
which we dereference to get ubik_currentTrans->type. ubik_currentTrans
could be set to NULL while this code is running, so there is a small
chance of this code causing a segfault, if SVOTE_Debug() is running
when the current transaction ends.
We only ever initialize ubik_currentTrans as a write transation (via
SDISK_Begin), so this check is pointless anyway. Accordingly, skip the
type check, and always assume that any active transaction is a write
transaction. This means we only ever access ubik_currentTrans once,
avoiding any risk of the value changing between accesses (and we no
longer need to dereference it, anyway).
Note that, since ubik_currentTrans is not marked as 'volatile', some C
compilers, with certain options, can and do assume that its value will
not change between accesses, and thus only fetch the pointer value once.
This avoids the risk of NULL dereference (and thus, crash, if pointer
stores/loads are atomic), but the value pointed to by ubik_currentTrans->type
would be incorrect when the transaction ends during the execution of
SVOTE_Debug().
Change-Id: Ia36c58e5906f5e8df59936f845ae11e886e8ec38
Reviewed-on: https://gerrit.openafs.org/13915
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
There is a perhaps-surprisingly large amount of code disabled behind
directives like '#if 0', '#ifdef notdef', and '#ifdef notyet'. At
best, this code is clutter, and at worst some of it is
confusing/outdated, and/or confusingly nested inside other
preprocessor conditionals. Sometimes this disabled code shows up when
grepping the tree, and causes a nuisance when refactoring related
areas of code.
Get rid of all of it. If anyone ever wants this code back, it can
always be restored by reverting portions of this commit.
Also delete some comments that clearly refer to the disabled code, and
in some cases, adjust the adjacent comments to make sense accordingly.
This commit doesn't touch any files in src/external/.
Change-Id: If260a41257e8d107930bd3c177eddb8ab336f0d1
Reviewed-on: https://gerrit.openafs.org/13683
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Move the functionality in afs_pthread_setname_self from libutil to
opr, in a new function opr_threadname_set. This allows us to more
easily use the routine in more subsystems, since most code already
uses opr.
Change-Id: I79d49617a19cd292a3b09ccfd9c9f319355a184e
Reviewed-on: https://gerrit.openafs.org/13655
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The determination of the CellServDB pathname is platform-dependent.
However, error reporting in the current code base assumes the CellServDB
location is platform-independent.
Add the pathname of the CellServDB file to the configuration directory
structure and set the new cellservDB member when opening the
configuration. Use this value when checking if the CellServDB has
changed and update the callers to use the cellservDB member when
reporting errors about the CellServDB file.
Change-Id: I5a3393fb9d4ae3c637d5a0d773598115314bfe1c
Reviewed-on: https://gerrit.openafs.org/13408
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Instead of using malloc and initializing various fields to 0, allocate
our ubik_dbase using calloc, to more easily ensure all fields are
initialized.
Change-Id: I5c2f345a82a2eb73d53ffc3e1b0fa408af6a8311
Reviewed-on: https://gerrit.openafs.org/13363
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Several areas in the code do something like this whenever the database
version is changed:
#ifdef AFS_PTHREAD_ENV
opr_cv_broadcast(&ubik_dbase->version_cond);
#else
LWP_NoYieldSignal(&ubik_dbase->version);
#endif
However, ever since commit 3fae4ea1 (ubik: remove unused code),
nothing in the tree waits for this condvar, so it currently doesn't do
anything. Remove this unneeded code.
Change-Id: I6903ed89f9dcee2ce154be8883d656d297c97902
Reviewed-on: https://gerrit.openafs.org/13361
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The ubik SendFile function performs a sanity check of the host address
before proceeding with the file transfer. Currently this check reuses
the file offset local variable to hold the value of the sync site
address, a 32-bit IPv4 address. Not only is this confusing, but also
causes a signed/unsigned type mismatch when comparing host addresses.
Instead of being so stingy with local variables, declare a new local
variable of the correct type to hold the value of the sync site address.
This separation is also a prerequisite for supporting larger address
types in the future.
Change-Id: I116fe210f418e6914afeff37c44d30bf795e2413
Reviewed-on: https://gerrit.openafs.org/13351
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
ubik_GetVersion and ubik_WaitVersion have been unused since at least
OpenAFS 1.0. Remove them.
No functional change should be incurred by this commit.
Change-Id: Iee6952f35d8c34e9f05a4e6011f5795f7222fb08
Reviewed-on: https://gerrit.openafs.org/13325
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The ubik library was written before positional i/o was available and
issues an lseek system call for each database file read and write. This
change converts the ubik database accesses to use positional i/o on
platforms where pread and pwrite are available, in order to reduce
system call load.
The new inline uphys_pread and uphys_pwrite functions are supplied on
platforms which do not supply pread and pwrite. These functions fall
back to non-positional i/o. If these symbols are present in the database
server binary then the server process will continue to call lseek before
each read and write access of the database file.
This change does not affect the whole-file database synchronization done
by ubik during database recovery (via the DISK_SendFile and DISK_GetFile
RPCs), which still uses non-positional i/o. However, that code does not
share file descriptors with the phys.c code, so there is no possibility
of mixing positional and non-positional i/o on the same FDs.
Change-Id: I28accd24f7f27b5e8a4f1dd0e3e08bab033c16e0
Reviewed-on: https://gerrit.openafs.org/12272
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
The ubik database file access layer has a file descriptor cache to avoid
reopening the database file on each file access. However, the file
offset is reset with lseek on each and every use of the cached file
descriptor, and the file offset is set twice when reading or writing
data records.
This change removes unnecessary and duplicate lseek system calls to
reduce the system call load.
Change-Id: I460b226d81e4eb64dc87918175acab495aa698cd
Reviewed-on: https://gerrit.openafs.org/12271
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Currently, when we write ubik i/o operations to the db log, we tend to
issue several syscalls involving small writes and fstat()s. This is
because each "log" operation involves at least one write, and each log
operation tends to be pretty small.
Each logged operation hitting disk separately is unnecessary, since
the db log does not need to hit the disk at all until we are ready to
commit the transaction. So to reduce the number of syscalls when
writing to the db, change our log writes to be buffered in memory
(using stdio calls). This also avoids needing to fstat() the
underlying log file, since we open the underlying file in append-only
mode, since we only ever append to (and truncate) the log file.
To implement this, we introduce a new 'buffered_append' phys
operation, to explicitly separate our buffered and non-buffered
operations, to try to avoid any bugs from mixing buffered and
non-buffered i/o. This new operation is only used for the db log.
Change-Id: I5596117c6c71ab7c2d552f71b0ef038f387e358a
Reviewed-on: https://gerrit.openafs.org/13070
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Joe Gorse <jhgorse@gmail.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
VOTE_Debug and VOTE_XDebug (udebug) both leave a single field
uninitialized if there is no current transaction. This leaks the memory
contents of the ubik server over the wire.
struct ubik_debug
- 4 bytes in member writeTrans
In common code to both RPCs, ensure that writeTrans is always
initialized.
[kaduk@mit.edu: switch to memset]
Change-Id: I91184b4ed0c159982a883ebaa9634406400eae93
Use the AFS_UNREACHED macro to suppress statement not reached warnings while
building under Solaris Studio. These warnings are emitted for statements
following functions declared with the noreturn function attribute.
Change-Id: Ic18cbb3ea78124acbe69edc0eccb2473b46648fe
Reviewed-on: https://gerrit.openafs.org/13010
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The value of errno can change after a syscall, and ViceLog may issue
syscalls (such as write()). So, make sure we save errno here before
calling ViceLog().
Issue spotted by kaduk@mit.edu.
Change-Id: I0f3308d64cd779bd97c97834ec2b270f5edd7ba6
Reviewed-on: https://gerrit.openafs.org/13263
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
As an aid for debugging database synchronization issues, ensure that the
logging is consistent and unambiguous for both the client and server
sides of DISK_GetFile and DISK_SendFile. Add new error messages as
required.
In addition, rework the "recovery sending version to <IP>" message in
urecovery_Interact. This message is misleading because the new version
database is only sent to a DB server if its version is not up to date.
Instead, move this message into the version check block immediately
below it. Also reword it for clarity and promote its log level from 5
to 0. Finally, remove the now-superfluous "recovery stating local
database" log message.
Change-Id: If8bbaa1215cab9fd24b157a0ee57759b34e77e9c
Reviewed-on: https://gerrit.openafs.org/13079
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
As a troubleshooting aid for developers, add a few counters and a log
msg so we know when transactions are being aborted (if any) by
urecovery_AbortAll.
Change-Id: I528df6d51acd5d10bb2de30f43b8d4415adc7f8a
Reviewed-on: https://gerrit.openafs.org/12618
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Many important ubik messages (e.g., errors, warnings, sync state
changes) are logged at log level 5 (-d 5) or higher. Many sites are
reluctant to run ubik servers at a logging level higher than the default
due to the large number of extremely noisy informational messages at log
level 5. Therefore, many important log messages are never seen.
Instead, issue critical errors, warnings, and other important messages
at log level 0 so that they are always seen, even at the default logging
level.
In addition, disambiguate the two "I am no longer sync-site" messages by
adding a unique reason text to each.
Change-Id: I057edf01e2502e39c5135836f1d0081d03559270
Reviewed-on: https://gerrit.openafs.org/12617
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
In order to better manage voting and recovery, each ubik server tracks
(in array ubik_servers) which of its fellow quorum members are 'up' or
not. However, ubik currently logs only when a server is "back up"; that
is, ubik_server->up transitions from 0 to 1.
Add new log messages to identify the time and reason when a server is
"marked down" (i.e., ubik_server->up transitions from 1 to 0).
Also modify two existing messages to have consistent wording with the
new "marked down" messages. Also change them to ViceLog (log level
0) so they will always be logged.
Change-Id: I29ee93e96cb7b28b943171d1477671c540a10d78
Reviewed-on: https://gerrit.openafs.org/12616
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Several logging statements in ubik contain an assignment statement
within the logging function call argument list, which would set a
variable as side effect of evaluating the function call arguments.
These embedded assignments are problematic since the logging function
calls have been replaced by ViceLog macros, which avoid the overhead of
a function call depending on logging levels.
Remove the embedded assignments within the logging argument lists so the
variables are always set regardless of the logging level.
Change-Id: Ifc0f32df2d01f9d8105b49e2c56a95758b184449
Reviewed-on: https://gerrit.openafs.org/13211
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Joe Gorse <jhgorse@gmail.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Most of the ContactQuorum_* routines are only used in ubik.c, so make
them all static - except for ContactQuorum_DISK_SetVersion, which is
called from disk.c.
Change-Id: I7d1ccd839e01ea8ee8d768dd369a892773361b05
Reviewed-on: https://gerrit.openafs.org/13078
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
This function is not used; remove it.
No functional change is incurred by this commit.
Change-Id: I7e3bb26fb62b0e28c8703154eb3df384d4dbc32d
Reviewed-on: https://gerrit.openafs.org/13077
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Ubik issues the same message in two very different cases:
- sync server issues DISK_GetFile to obtain the latest version
- non-sync server receives DISK_SendFile from the sync server
Modify the messages so they provide more information and are
distinguishable from each other.
Change-Id: I99e8adc7229260f478a0df15791216e090d2e113
Reviewed-on: https://gerrit.openafs.org/12615
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Use the server logging macros instead of the utility functions to avoid
function call overhead, especially at logging level 25. The server
logging macros perform a logging level check in-line to avoid the
unnecessary ubik_dprint* calls.
Change-Id: Ia86efad6257b764f0922957017fe8326f0de76d3
Reviewed-on: https://gerrit.openafs.org/12619
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
When udisk_write is extending the database, DRead will return a null
buffer. udisk_write then calls DNew to get a brand new buffer for the
extension write, and clears it with memset. However, this is redundant,
since DNew has already cleared the new buffer.
Remove the redundant memset.
No functional change should be incurred by this commit.
Change-Id: Ia6768098fb3c67475c8948c874b92b91bf17cdb7
Reviewed-on: https://gerrit.openafs.org/12621
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
ubik has a few very old "orphaned" LWP events that are signalled via
LWP_NoYieldSignal, but have no matching waits (LWP_WaitProcess).
Each "signal" runs the LWP waiting element list for each LWP on the
blocked queue; this may add up to substantial wasted overhead on a
heavily loaded ubik server.
Remove the orphaned signals.
No functional difference should be incurred by this commit.
Change-Id: I66eba45975a829216e7af1927e51ec6aab63f570
Reviewed-on: https://gerrit.openafs.org/12620
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Nothing uses the udisk_Log* functions outside of disk.c. Declare these
static to make sure they stay that way, to make it easier to change
their semantics.
Change-Id: I068684782b22af788ce892c995a6d80f2d9fb2e0
Reviewed-on: https://gerrit.openafs.org/13069
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Nothing uses the 'mtime' field from ubik_stat. Remove it.
Change-Id: I7611a7ca5aa5743be43aefafeda5ecf9a5d47598
Reviewed-on: https://gerrit.openafs.org/13068
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Clones should not be able to become the sync-site. To make it possible,
regular sites do not vote for a site tagged as clone. In other words,
the clones ask for votes but they cannot be the sync-site. Knowing that
their requests for votes should be refused by the regular sites, they
should never have enough votes to win the election.
In addition to the unnecessary network traffic created by these
unnecessary requests, this current approach can be problematic in some
specific situations. As an example, consider the following scenario:
The user wants to turn a regular site, called host1, into a clone.
To do so, he runs the following commands on every single server:
$ bos removehost -server <server> -host host1
$ bos addhost -server <server> -host host1 -clone
After that, he restarts the servers, one by one. Depending on the delay
between the restarts, a clone can become the sync-site. This is possible
because the clones request votes from the other sites. If enough regular
sites are not aware (yet) that the request for vote came from a clone,
the clone in question can get enough votes to win the election.
To fix the problems mentioned above, do not request votes if you cannot
be the sync-site.
Change-Id: Ic3569af8264dfff32f2a86b8dd99b922193f010a
Reviewed-on: https://gerrit.openafs.org/12654
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Solaris Studio issues warnings for statements which can not be reached,
such as statements following an infinite loop. For example, the return
statement will generate a 'statement not reached' warning in the
following code:
while (1) {
/* no breaks or gotos in this body */
}
return 0;
Suppress these warnings by conditionally removing such statements when
building under Solaris Studio.
Change-Id: Ib4f465bf9c00eff0d603e5bd643db7d3a5aa0ba0
Reviewed-on: https://gerrit.openafs.org/12958
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
In SDISK_SendFile, we return a USYNC error if the caller is not the
sync site. Say who the sync site is when we do this, to possibly help
post-mortem debugging.
Change-Id: I62a3565fca20171be20481638c261c4659c68ab2
Reviewed-on: https://gerrit.openafs.org/12943
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
