Skip to content

Commit

Permalink
simplify
Browse files Browse the repository at this point in the history
  • Loading branch information
miagilepner committed May 23, 2023
1 parent b5c5450 commit 3567ce6
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 72 deletions.
33 changes: 15 additions & 18 deletions vault/activity_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ func (a *ActivityLog) saveCurrentSegmentToStorageLocked(ctx context.Context, for
}
a.currentSegment.currentClients.Clients = segmentClients

_, err := a.saveSegmentInternal(ctx, a.currentSegment, force)
err := a.saveCurrentSegmentInternal(ctx, force)
if err != nil {
// The current fragment(s) have already been placed into the in-memory
// segment, but we may lose any excess (in excessClients).
Expand All @@ -384,7 +384,7 @@ func (a *ActivityLog) saveCurrentSegmentToStorageLocked(ctx context.Context, for
excessClients = excessClients[:activitySegmentClientCapacity]
}
a.currentSegment.currentClients.Clients = excessClients
_, err := a.saveSegmentInternal(ctx, a.currentSegment, force)
err := a.saveCurrentSegmentInternal(ctx, force)
if err != nil {
return err
}
Expand All @@ -393,24 +393,23 @@ func (a *ActivityLog) saveCurrentSegmentToStorageLocked(ctx context.Context, for
}

// :force: forces a save of tokens/entities even if the in-memory log is empty
func (a *ActivityLog) saveSegmentInternal(ctx context.Context, currentSegment segmentInfo, force bool) ([]string, error) {
entityPath := fmt.Sprintf("%s%d/%d", activityEntityBasePath, currentSegment.startTimestamp, currentSegment.clientSequenceNumber)
func (a *ActivityLog) saveCurrentSegmentInternal(ctx context.Context, force bool) error {
entityPath := fmt.Sprintf("%s%d/%d", activityEntityBasePath, a.currentSegment.startTimestamp, a.currentSegment.clientSequenceNumber)
// RFC (VLT-120) defines this as 1-indexed, but it should be 0-indexed
tokenPath := fmt.Sprintf("%s%d/0", activityTokenBasePath, currentSegment.startTimestamp)
paths := make([]string, 0, 2)
tokenPath := fmt.Sprintf("%s%d/0", activityTokenBasePath, a.currentSegment.startTimestamp)

for _, client := range a.currentSegment.currentClients.Clients {
// Explicitly catch and throw clear error message if client ID creation and storage
// results in a []byte that doesn't assert into a valid string.
if !utf8.ValidString(client.ClientID) {
return nil, fmt.Errorf("client ID %q is not a valid string:", client.ClientID)
return fmt.Errorf("client ID %q is not a valid string:", client.ClientID)
}
}

if len(currentSegment.currentClients.Clients) > 0 || force {
clients, err := proto.Marshal(currentSegment.currentClients)
if len(a.currentSegment.currentClients.Clients) > 0 || force {
clients, err := proto.Marshal(a.currentSegment.currentClients)
if err != nil {
return nil, err
return err
}

a.logger.Trace("writing segment", "path", entityPath)
Expand All @@ -419,15 +418,14 @@ func (a *ActivityLog) saveSegmentInternal(ctx context.Context, currentSegment se
Value: clients,
})
if err != nil {
return nil, err
return err
}
paths = append(paths, entityPath)
}

// We must still allow for the tokenCount of the current segment to
// be written to storage, since if we remove this code we will incur
// data loss for one segment's worth of TWEs.
if len(currentSegment.tokenCount.CountByNamespaceID) > 0 || force {
if len(a.currentSegment.tokenCount.CountByNamespaceID) > 0 || force {
// We can get away with simply using the oldest version stored because
// the storing of versions was introduced at the same time as this code.
oldestVersion, oldestUpgradeTime, err := a.core.FindOldestVersionTimestamp()
Expand All @@ -444,7 +442,7 @@ func (a *ActivityLog) saveSegmentInternal(ctx context.Context, currentSegment se
}
tokenCount, err := proto.Marshal(a.currentSegment.tokenCount)
if err != nil {
return nil, err
return err
}

a.logger.Trace("writing segment", "path", tokenPath)
Expand All @@ -453,11 +451,10 @@ func (a *ActivityLog) saveSegmentInternal(ctx context.Context, currentSegment se
Value: tokenCount,
})
if err != nil {
return nil, err
return err
}
paths = append(paths, tokenPath)
}
return paths, nil
return nil
}

// parseSegmentNumberFromPath returns the segment number from a path
Expand Down Expand Up @@ -1024,7 +1021,7 @@ func (a *ActivityLog) SetConfig(ctx context.Context, config activityConfig) {

if forceSave {
// l is still held here
a.saveSegmentInternal(ctx, a.currentSegment, true)
a.saveCurrentSegmentInternal(ctx, true)
}

a.defaultReportMonths = config.DefaultReportMonths
Expand Down
55 changes: 1 addition & 54 deletions vault/logical_system_activity_write_testonly.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,34 +54,7 @@ func (b *SystemBackend) handleActivityWriteData(ctx context.Context, request *lo
if len(input.Data) == 0 {
return logical.ErrorResponse("Missing required \"data\" values"), logical.ErrInvalidRequest
}

numMonths := 0
for _, month := range input.Data {
if int(month.GetMonthsAgo()) > numMonths {
numMonths = int(month.GetMonthsAgo())
}
}
generated := newMultipleMonthsActivityClients(numMonths)
for _, month := range input.Data {
err := generated.processMonth(ctx, b.Core, month)
if err != nil {
return logical.ErrorResponse("failed to process data for month %d", month.GetMonthsAgo()), err
}
}

opts := make(map[generation.WriteOptions]struct{}, len(input.Write))
for _, opt := range input.Write {
opts[opt] = struct{}{}
}
paths, err := generated.write(ctx, opts, b.Core.activityLog)
if err != nil {
return logical.ErrorResponse("failed to write data"), err
}
return &logical.Response{
Data: map[string]interface{}{
"paths": paths,
},
}, nil
return nil, nil
}

// singleMonthActivityClients holds a single month's client IDs, in the order they were seen
Expand Down Expand Up @@ -317,32 +290,6 @@ func (m *multipleMonthsActivityClients) addRepeatedClients(monthsAgo int32, c *g
return nil
}

func (m *multipleMonthsActivityClients) write(ctx context.Context, opts map[generation.WriteOptions]struct{}, activityLog *ActivityLog) ([]string, error) {
paths := []string{}
for _, month := range m.months {
segments := month.populateSegments()
timestamp := 0
for segmentIndex, segment := range segments {
if _, ok := opts[generation.WriteOptions_WRITE_ENTITIES]; ok {
if segment == nil {
// skip the index
continue
}
entityPaths, err := activityLog.saveSegmentInternal(ctx, segmentInfo{
startTimestamp: int64(timestamp),
currentClients: &activity.EntityActivityLog{Clients: segment},
clientSequenceNumber: uint64(segmentIndex),
}, true)
if err != nil {
return nil, err
}
paths = append(paths, entityPaths...)
}
}
}
return paths, nil
}

func newMultipleMonthsActivityClients(numberOfMonths int) *multipleMonthsActivityClients {
m := &multipleMonthsActivityClients{
months: make([]*singleMonthActivityClients, numberOfMonths),
Expand Down

0 comments on commit 3567ce6

Please sign in to comment.