Blair Zajac wrote:
> On 04/15/2012 04:20 AM, stefan2_at_apache.org wrote:
>> Author: stefan2
>> Date: Sun Apr 15 11:20:58 2012
>> New Revision: 1326307
>>
>> URL: http://svn.apache.org/viewvc?rev=1326307&view=rev
>> Log:
>> Merge all changes (-r1298521-1326293) from branches/revprop-cache to
>> trunk
>> and resolve minor conflicts.
>
>> +/* Test whether revprop cache and necessary infrastructure are
>> + available in FS. */
>> +static svn_boolean_t
>> +has_revprop_cache(svn_fs_t *fs)
>> +{
>> + fs_fs_data_t *ffd = fs->fsap_data;
>> + svn_error_t *error;
>> +
>> + /* is the cache (still) enabled? */
>> + if (ffd->revprop_cache == NULL)
>> + return FALSE;
>> +
>> + /* try to access our SHM-backed infrastructure */
>> + error = ensure_revprop_generation(fs);
>> + if (error)
>> + {
>> + /* failure -> disable revprop cache for good */
>> +
>> + svn_error_clear(error);
>
> Should we log why it was disabled instead of silently doing so? I
> would want a way to determine why it was disabled.
>
What logging API should I call?
> How about returning the error to the caller so they can log it? They
> can check the svn_error_t == SVN_NO_ERROR to see if it's enabled or not.
>
I don't think that the answer to the question
"shall I use the revprop cache?" should be
"does not compute!" instead of a simple "no."
>> +/* Read the current revprop generation and return it in *GENERATION.
>> + Also, detect aborted / crashed writers and recover from that.
>> + Use the access object in FS to set the shared mem values. */
>> +static svn_error_t *
>> +read_revprop_generation(svn_fs_t *fs,
>> + apr_int64_t *generation)
>
> Out parameters are normally listed first.
>
Fixed in r1326384.
Thanks for the review!
-- Stefan^2.
Received on 2012-04-15 19:29:02 CEST