Index options updates fallback to normal build instead of proceeding with delta build (#376) · boltless.me/zoekt@e51248f

+44 -9

build/builder.go

··· 104 104 changedOrRemovedFiles []string 105 105 } 106 106 107 - // HashOptions creates a hash of the options that affect an index. 108 - func (o *Options) HashOptions() string { 107 + // HashOptions contains only the options in Options that upon modification leads to IndexState of IndexStateMismatch during the next index building. 108 + type HashOptions struct { 109 + sizeMax int 110 + disableCTags bool 111 + cTags string 112 + cTagsMustSucceed bool 113 + largeFiles []string 114 + } 115 + 116 + func (o *Options) HashOptions() HashOptions { 117 + return HashOptions{ 118 + sizeMax: o.SizeMax, 119 + disableCTags: o.DisableCTags, 120 + cTags: o.CTags, 121 + cTagsMustSucceed: o.CTagsMustSucceed, 122 + largeFiles: o.LargeFiles, 123 + } 124 + } 125 + 126 + func (o *Options) GetHash() string { 127 + h := o.HashOptions() 109 128 hasher := sha1.New() 110 129 111 - hasher.Write([]byte(o.CTags)) 112 - hasher.Write([]byte(fmt.Sprintf("%t", o.CTagsMustSucceed))) 113 - hasher.Write([]byte(fmt.Sprintf("%d", o.SizeMax))) 114 - hasher.Write([]byte(fmt.Sprintf("%q", o.LargeFiles))) 115 - hasher.Write([]byte(fmt.Sprintf("%t", o.DisableCTags))) 130 + hasher.Write([]byte(h.cTags)) 131 + hasher.Write([]byte(fmt.Sprintf("%t", h.cTagsMustSucceed))) 132 + hasher.Write([]byte(fmt.Sprintf("%d", h.sizeMax))) 133 + hasher.Write([]byte(fmt.Sprintf("%q", h.largeFiles))) 134 + hasher.Write([]byte(fmt.Sprintf("%t", h.disableCTags))) 116 135 117 136 return fmt.Sprintf("%x", hasher.Sum(nil)) 118 137 } ··· 355 374 return IndexStateCorrupt, fn 356 375 } 357 376 358 - if repo.IndexOptions != o.HashOptions() { 377 + if repo.IndexOptions != o.GetHash() { 359 378 return IndexStateOption, fn 360 379 } 361 380 ··· 655 674 } 656 675 } 657 676 677 + if b.opts.GetHash() != repository.IndexOptions { 678 + return &deltaIndexOptionsMismatchError{ 679 + shardName: shard, 680 + newOptions: b.opts.HashOptions(), 681 + } 682 + } 683 + 658 684 repository.Branches = b.opts.RepositoryDescription.Branches 659 685 660 686 repository.LatestCommitDate = b.opts.RepositoryDescription.LatestCommitDate ··· 944 970 desc := b.opts.RepositoryDescription 945 971 desc.HasSymbols = b.opts.CTags != "" 946 972 desc.SubRepoMap = b.opts.SubRepositories 947 - desc.IndexOptions = b.opts.HashOptions() 973 + desc.IndexOptions = b.opts.GetHash() 948 974 949 975 shardBuilder, err := zoekt.NewIndexBuilder(&desc) 950 976 if err != nil { ··· 996 1022 997 1023 func (e deltaBranchSetError) Error() string { 998 1024 return fmt.Sprintf("repository metadata in shard %q contains a different set of branch names than what was requested, which is unsupported in a delta shard build. old: %+v, new: %+v", e.shardName, e.old, e.new) 1025 + } 1026 + 1027 + type deltaIndexOptionsMismatchError struct { 1028 + shardName string 1029 + newOptions HashOptions 1030 + } 1031 + 1032 + func (e *deltaIndexOptionsMismatchError) Error() string { 1033 + return fmt.Sprintf("one or more index options for shard %q do not match Builder's index options. These index option updates are incompatible with delta build. New index options: %+v", e.shardName, e.newOptions) 999 1034 } 1000 1035 1001 1036 // umask holds the Umask of the current process

+61

build/builder_test.go

··· 456 456 } 457 457 } 458 458 459 + func TestBuilder_DeltaShardsBuildsShouldErrorOnIndexOptionsMismatch(t *testing.T) { 460 + repository := zoekt.Repository{ 461 + Name: "repo", 462 + ID: 1, 463 + Branches: []zoekt.RepositoryBranch{{Name: "foo"}}, 464 + } 465 + 466 + for _, test := range []struct { 467 + name string 468 + options func(options *Options) 469 + }{ 470 + { 471 + name: "update option CTags to non default", 472 + options: func(options *Options) { options.CTags = "ctags_updated_test" }, 473 + }, 474 + { 475 + name: "update option DisableCTags to non default", 476 + options: func(options *Options) { options.DisableCTags = true }, 477 + }, 478 + { 479 + name: "update option SizeMax to non default", 480 + options: func(options *Options) { options.SizeMax -= 10 }, 481 + }, 482 + { 483 + name: "update option LargeFiles to non default", 484 + options: func(options *Options) { options.LargeFiles = []string{"-large_file", "*.md", "-large_file", "*.yaml"} }, 485 + }, 486 + } { 487 + test := test 488 + 489 + t.Run(test.name, func(t *testing.T) { 490 + indexDir := t.TempDir() 491 + 492 + // initially use default options 493 + createTestShard(t, indexDir, repository, 2) 494 + 495 + o := Options{ 496 + IndexDir: indexDir, 497 + RepositoryDescription: repository, 498 + IsDelta: true, 499 + } 500 + test.options(&o) 501 + 502 + b, err := NewBuilder(o) 503 + if err != nil { 504 + t.Fatalf("NewBuilder: %v", err) 505 + } 506 + 507 + err = b.Finish() 508 + if err == nil { 509 + t.Fatalf("no error regarding index options mismatch") 510 + } 511 + 512 + var optionsMismatchError *deltaIndexOptionsMismatchError 513 + if !errors.As(err, &optionsMismatchError) { 514 + t.Fatalf("expected error complaining about index options mismatch, got: %s", err) 515 + } 516 + }) 517 + } 518 + } 519 + 459 520 func TestBuilder_DeltaShardsMetadataInOlderShards(t *testing.T) { 460 521 olderTime := time.Unix(0, 0) 461 522 newerTime := time.Unix(10000, 0)

+6

gitindex/index.go

··· 627 627 return nil, nil, nil, nil, fmt.Errorf("requested branch set in build options (%q) != branch set found on disk (%q) - branch set must be the same for delta shards", optionsBranchList, existingBranchList) 628 628 } 629 629 630 + // Check if the build options hash does not match the repository metadata's hash 631 + // If it does not match then one or more index options has changed and will require a normal build instead of a delta build 632 + if options.BuildOptions.GetHash() != existingRepository.IndexOptions { 633 + return nil, nil, nil, nil, fmt.Errorf("one or more index options previously stored for repository %s (ID: %d) does not match the index options for this requested build; These index option updates are incompatible with delta build. new index options: %+v", existingRepository.Name, existingRepository.ID, options.BuildOptions.HashOptions()) 634 + } 635 + 630 636 // branch => (path, sha1) => repo. 631 637 repos = map[fileKey]BlobLocation{} 632 638

+40

gitindex/index_test.go

··· 363 363 }, 364 364 }, 365 365 { 366 + name: "should fallback to normal build if one or more index options updates requires a full build", 367 + branches: []string{"main"}, 368 + steps: []step{ 369 + { 370 + name: "setup", 371 + addedDocuments: branchToDocumentMap{ 372 + "main": []zoekt.Document{fruitV1}, 373 + }, 374 + 375 + expectedDocuments: []zoekt.Document{fruitV1}, 376 + }, 377 + { 378 + name: "try delta build after updating Disable CTags index option", 379 + addedDocuments: branchToDocumentMap{ 380 + "main": []zoekt.Document{fruitV2}, 381 + }, 382 + optFn: func(t *testing.T, o *Options) { 383 + o.BuildOptions.IsDelta = true 384 + o.BuildOptions.DisableCTags = true 385 + }, 386 + 387 + expectedFallbackToNormalBuild: true, 388 + expectedDocuments: []zoekt.Document{fruitV2}, 389 + }, 390 + { 391 + name: "try delta build after reverting Disable CTags index option", 392 + addedDocuments: branchToDocumentMap{ 393 + "main": []zoekt.Document{fruitV3}, 394 + }, 395 + optFn: func(t *testing.T, o *Options) { 396 + o.BuildOptions.IsDelta = true 397 + o.BuildOptions.DisableCTags = false 398 + }, 399 + 400 + expectedFallbackToNormalBuild: true, 401 + expectedDocuments: []zoekt.Document{fruitV3}, 402 + }, 403 + }, 404 + }, 405 + { 366 406 name: "should fallback to normal build if repository has unsupported Sourcegraph ignore file", 367 407 branches: []string{"main"}, 368 408 steps: []step{

Configure Feed

Configure Feed