Deduplication --> Antispam #293

phbnf · 2025-05-08T17:15:51Z

Towards #292

While I'm there, fix the aws test deployment instructions.

# Conflicts: # cmd/gcp/main.go

# Conflicts: # go.mod # go.sum # Conflicts: # go.sum # Conflicts: # go.sum

# Conflicts: # deployment/modules/aws/tesseract/conformance/main.tf

roger2hk

There are dedup related variables to be deleted in the deployment directory.

roger2hk · 2025-05-08T17:23:45Z

deployment/live/aws/test/README.md

-  2. Run `terragrunt apply`.
-
+  2. Run `terragrunt apply`. If this fails to create the antispam database,
+  connect the RDS instance to your VM using the instrunctions bellow, and run


Suggested change

connect the RDS instance to your VM using the instrunctions bellow, and run

connect the RDS instance to your VM using the instructions below, and run

roger2hk · 2025-05-08T17:24:30Z

deployment/modules/aws/storage/main.tf

+# creating the aws_db_instance.
+# This requires that the machine running terraform has access
+# to the DB instance created above. This is _NOT_ the case when
+# github actions are applying the terraform.


Suggested change

# github actions are applying the terraform.

# GitHub actions are applying the terraform.

roger2hk · 2025-05-08T17:27:39Z

cmd/gcp/main.go

@@ -53,7 +54,8 @@ var (
 	origin                     = flag.String("origin", "", "Origin of the log, for checkpoints and the monitoring prefix.")
 	bucket                     = flag.String("bucket", "", "Name of the bucket to store the log in.")
 	spannerDB                  = flag.String("spanner_db_path", "", "Spanner database path: projects/{projectId}/instances/{instanceId}/databases/{databaseId}.")
-	spannerDedupDB             = flag.String("spanner_dedup_db_path", "", "Spanner deduplication database path: projects/{projectId}/instances/{instanceId}/databases/{databaseId}.")
+	spannerAntispamDB          = flag.String("spanner_antispam_db_path", "", "EXPERIMENTAL: Spanner antispam deduplication database path projects/{projectId}/instances/{instanceId}/databases/{databaseId}.")
+	inMemoryAntispamCacheSize  = flag.Uint("inmemory_antispam_cache_size", 2<<10, "Maximum number of entries to keep in the in-memory antispam cache.")


Suggested change

inMemoryAntispamCacheSize = flag.Uint("inmemory_antispam_cache_size", 2<<10, "Maximum number of entries to keep in the in-memory antispam cache.")

inmemoryAntispamCacheSize = flag.Uint("inmemory_antispam_cache_size", 2<<10, "Maximum number of entries to keep in the in-memory antispam cache.")

roger2hk · 2025-05-08T17:27:59Z

cmd/aws/main.go

 	dbHost                     = flag.String("db_host", "", "AuroraDB host")
 	dbPort                     = flag.Int("db_port", 3306, "AuroraDB port")
 	dbUser                     = flag.String("db_user", "", "AuroraDB user")
 	dbPassword                 = flag.String("db_password", "", "AuroraDB password")
 	dbMaxConns                 = flag.Int("db_max_conns", 0, "Maximum connections to the database, defaults to 0, i.e unlimited")
 	dbMaxIdle                  = flag.Int("db_max_idle_conns", 2, "Maximum idle database connections in the connection pool, defaults to 2")
-	dedupPath                  = flag.String("dedup_path", "", "Path to the deduplication database.")
+	inMemoryAntispamCacheSize  = flag.Uint("inmemory_antispam_cache_size", 2<<10, "Maximum number of entries to keep in the in-memory antispam cache.")


Suggested change

inMemoryAntispamCacheSize = flag.Uint("inmemory_antispam_cache_size", 2<<10, "Maximum number of entries to keep in the in-memory antispam cache.")

inmemoryAntispamCacheSize = flag.Uint("inmemory_antispam_cache_size", 2<<10, "Maximum number of entries to keep in the in-memory antispam cache.")

roger2hk · 2025-05-08T17:28:18Z

cmd/aws/main.go

+	if *antispamDBName != "" {
+		antispam, err = aws_as.NewAntispam(ctx, antispamMysqlConfig().FormatDSN(), aws_as.AntispamOpts{})
+		if err != nil {
+			klog.Exitf("Failed to create new GCP antispam storage: %v", err)


Suggested change

klog.Exitf("Failed to create new GCP antispam storage: %v", err)

klog.Exitf("Failed to create new AWS antispam storage: %v", err)

roger2hk · 2025-05-08T17:30:26Z

cmd/aws/main.go

+	if *dbPort == 0 {
+		klog.Exit("--db_port must be set")
+	}


The default dbPort is 3306 instead of 0.

Yes, this is working as intended: it's ok to use the default port, but 0 would indicate that it's the default integer.

If --db_port is not set, the *dbPort value would be 3306. The log message is not valid.

roger2hk · 2025-05-08T17:30:48Z

cmd/aws/main.go

+	if *dbUser == "" {
+		klog.Exit("--db_user must be set")
+	}
+	// Empty passord isn't an option with AuroraDB MySQL.


Suggested change

// Empty passord isn't an option with AuroraDB MySQL.

// Empty password isn't an option with AuroraDB MySQL.

roger2hk · 2025-05-08T17:31:19Z

deployment/live/aws/test/README.md

@@ -95,7 +99,9 @@ go run ./cmd/aws \
  --db_port=3306 \
  --db_user=tesseract \
  --db_password=${TESSERACT_DB_PASSWORD} \
-  --dedup_path=test-static-ct
+  --antispam_db_name=antispam_db


Suggested change

--antispam_db_name=antispam_db

--antispam_db_name=antispam_db \

roger2hk · 2025-05-08T17:31:27Z

deployment/live/aws/test/README.md

@@ -187,7 +193,9 @@ go run ./cmd/aws \
  --db_port=3306 \
  --db_user=tesseract \
  --db_password=${TESSERACT_DB_PASSWORD} \
-  --dedup_path=test-static-ct
+  --antispam_db_name=antispam_db


Suggested change

--antispam_db_name=antispam_db

--antispam_db_name=antispam_db \

roger2hk · 2025-05-08T17:33:57Z

deployment/modules/gcp/storage/main.tf

+  ddl = [
+    	"CREATE TABLE IF NOT EXISTS FollowCoord (id INT64 NOT NULL, nextIdx INT64 NOT NULL) PRIMARY KEY (id)",
+			"CREATE TABLE IF NOT EXISTS IDSeq (h BYTES(32) NOT NULL, idx INT64 NOT NULL) PRIMARY KEY (h)",
+  ]


Suggested change

ddl = [

"CREATE TABLE IF NOT EXISTS FollowCoord (id INT64 NOT NULL, nextIdx INT64 NOT NULL) PRIMARY KEY (id)",

"CREATE TABLE IF NOT EXISTS IDSeq (h BYTES(32) NOT NULL, idx INT64 NOT NULL) PRIMARY KEY (h)",

]

ddl = [

"CREATE TABLE IF NOT EXISTS FollowCoord (id INT64 NOT NULL, nextIdx INT64 NOT NULL) PRIMARY KEY (id)",

"CREATE TABLE IF NOT EXISTS IDSeq (h BYTES(32) NOT NULL, idx INT64 NOT NULL) PRIMARY KEY (h)",

]

roger2hk · 2025-05-08T17:50:53Z

cmd/aws/main.go

@@ -230,3 +236,32 @@ func storageConfigFromFlags() taws.Config {
 		MaxIdleConns: *dbMaxIdle,
 	}
 }
+
+func antispamMysqlConfig() *mysql.Config {


Suggested change

func antispamMysqlConfig() *mysql.Config {

func antispamMySQLConfig() *mysql.Config {

https://google.github.io/styleguide/go/decisions#initialisms

AlCutter · 2025-05-09T09:04:21Z

cmd/aws/main.go

 	dbHost                     = flag.String("db_host", "", "AuroraDB host")
 	dbPort                     = flag.Int("db_port", 3306, "AuroraDB port")
 	dbUser                     = flag.String("db_user", "", "AuroraDB user")
 	dbPassword                 = flag.String("db_password", "", "AuroraDB password")
 	dbMaxConns                 = flag.Int("db_max_conns", 0, "Maximum connections to the database, defaults to 0, i.e unlimited")
 	dbMaxIdle                  = flag.Int("db_max_idle_conns", 2, "Maximum idle database connections in the connection pool, defaults to 2")
-	dedupPath                  = flag.String("dedup_path", "", "Path to the deduplication database.")
+	inMemoryAntispamCacheSize  = flag.Uint("inmemory_antispam_cache_size", 2<<10, "Maximum number of entries to keep in the in-memory antispam cache.")


'2048' might well be enough, but I'd be tempted default to a larger size which can be tuned down later/via flag if need be.
I've been using 256k as a place-holder in the conformance bins (32+8 bytes of data ==> 10MB of raw cached data plus whatever overhead Go's map incurs)

AlCutter · 2025-05-09T09:09:27Z

cmd/gcp/main.go

@@ -53,7 +54,8 @@ var (
 	origin                     = flag.String("origin", "", "Origin of the log, for checkpoints and the monitoring prefix.")
 	bucket                     = flag.String("bucket", "", "Name of the bucket to store the log in.")
 	spannerDB                  = flag.String("spanner_db_path", "", "Spanner database path: projects/{projectId}/instances/{instanceId}/databases/{databaseId}.")
-	spannerDedupDB             = flag.String("spanner_dedup_db_path", "", "Spanner deduplication database path: projects/{projectId}/instances/{instanceId}/databases/{databaseId}.")
+	spannerAntispamDB          = flag.String("spanner_antispam_db_path", "", "EXPERIMENTAL: Spanner antispam deduplication database path projects/{projectId}/instances/{instanceId}/databases/{databaseId}.")


Do you want to remove the EXPERIMENTAL: warning here, or also add it to the aws flags? Should probably be consistent unless we do actually want to indicate there's a difference.

AlCutter · 2025-05-09T09:11:44Z

cmd/gcp/main.go

@@ -157,9 +159,19 @@ func newGCPStorage(ctx context.Context, signer note.Signer) (*storage.CTStorage,
 		return nil, fmt.Errorf("failed to initialize GCP Tessera storage driver: %v", err)
 	}

+	var antispam tessera.Antispam
+	// Persistent antispam is currently experimental, so there's no terraform or documentation yet!


nit: I think there might be terraform now (at least in Tessera), although docs may need updating :)

AlCutter · 2025-05-09T09:20:24Z

deployment/modules/gcp/cloudrun/main.tf

        "--roots_pem_file=/bin/test_root_ca_cert.pem",
        "--origin=${var.base_name}${var.origin_suffix}",
        "--signer_public_key_secret_name=${var.signer_public_key_secret_name}",
        "--signer_private_key_secret_name=${var.signer_private_key_secret_name}",
+      	"--inmemory_antispam_cache_size=25000000", # About 1GB of memory.


For potential future CL, wondering if something like https://pkg.go.dev/github.com/dustin/go-humanize might be helpful in making numerical flags understand more friendly representations of large numbers?

AlCutter · 2025-05-09T09:23:41Z

internal/ct/handlers.go

-		if err != nil {
-			klog.Warningf("AddCertIndex(): failed to store certificate index: %v", err)
+	klog.V(2).Infof("%s: %s => storage.Add", log.origin, method)
+	index, err := log.storage.Add(ctx, entry)()


Do you need to do something with index.IsDup and fetch the old timestamp if it's true?

AlCutter · 2025-05-12T13:13:56Z

storage/storage.go

+	awaiter := tessera.NewPublicationAwaiter(ctx, cts.reader.ReadCheckpoint, 10*time.Millisecond)
+	idx, cpRaw, err := awaiter.Await(ctx, f)
+	if err != nil {
+		return 0, 0, fmt.Errorf("error waiting for Tessera future and its integration: %v", err)
+	}


You should create a single instance of this (maybe in the storage?) and reuse it across all add requests, otherwise you'll have lots and lots of goroutines spun up, many requests for /checkpoint, etc.

Might be worth upping the loop time to something a bit higher, say 200-300ms? 10ms is fetching /checkpoint 100x per second!

Done. There's already a TODO somewhere to fine tune this value together with the request deadline. I've raised it to 200ms for now, and we can edit it later if need be.

AlCutter · 2025-05-12T13:18:55Z

storage/storage.go

+
+	eIdx := idx.Index % layout.EntryBundleWidth
+	if uint64(len(eb.Entries)) < eIdx {
+		return 0, 0, fmt.Errorf("failed to read entry %d in entry bundle %d", eIdx, eBIdx)


nit: entry bundle at index %d has only %d entries, but wanted at least %d

This is really a fatal log corruption failure; if ckptSize > eBIdx, then the EB must exist and have sufficient entries.

AlCutter · 2025-05-12T13:23:33Z

storage/storage.go

+	e := staticct.Entry{}
+	if err := e.UnmarshalText([]byte(eb.Entries[eIdx])); err != nil {
+		return 0, 0, fmt.Errorf("failed to unmarshal entry %d in entry bundle %d: %v", eIdx, eBIdx, e)
+	}


Maybe one to keep in the back-pocket, but you could just read the timestamp from the first 8 bytes here, and avoid reading/parsing the rest of the entry?

Oh yeah good point, the timestamp is at the beginning of the entry! I think we can do even smarter things here, since we already have to parse every entries in the bundle to get a to a specific entry since we don't know the size of every entry. I've left a TODO to make this better, once the APIs are a bit more mature and we know exactly what we need to read (as a matter of fact, it's changing in this PR!). The change you suggested was too small not to be implemented, so I just did it tough.

AlCutter · 2025-05-12T15:02:01Z

internal/types/staticct/staticct.go

+	if t > math.MaxInt64 {
+		return 0, fmt.Errorf("invalid data tile: timestamp %d > math.MaxInt64", t)
+	}


Did you mean to duplicate the check here?

phbnf added 2 commits May 8, 2025 08:07

enable new dedup

468acc1

# Conflicts: # cmd/gcp/main.go

don't do old deduplication

5de64b4

phbnf assigned roger2hk and AlCutter May 8, 2025

dedupeverywhere

b98391d

# Conflicts: # go.mod # go.sum # Conflicts: # go.sum # Conflicts: # go.sum

phbnf force-pushed the removeold branch from a3c6711 to ed1e3d8 Compare May 8, 2025 17:18

use flag for antispam size

48e9424

# Conflicts: # deployment/modules/aws/tesseract/conformance/main.tf

phbnf force-pushed the removeold branch from ed1e3d8 to 48e9424 Compare May 8, 2025 17:29

roger2hk reviewed May 8, 2025

View reviewed changes

address comments

2762ee9

phbnf marked this pull request as ready for review May 8, 2025 17:53

phbnf added 2 commits May 8, 2025 18:00

remove old dedup terraform and readme

4dd173e

remove dedup todo

6823b7b

roger2hk approved these changes May 8, 2025

View reviewed changes

AlCutter approved these changes May 9, 2025

View reviewed changes

phbnf force-pushed the removeold branch from aafb95c to c330eb4 Compare May 9, 2025 14:52

address comments

19c2c1d

phbnf force-pushed the removeold branch from c330eb4 to 19c2c1d Compare May 9, 2025 14:54

add TODOs

80b9ff1

AlCutter reviewed May 12, 2025

View reviewed changes

address comments

2d6da67

AlCutter reviewed May 12, 2025

View reviewed changes

remove duplicate check

5b8df52

AlCutter approved these changes May 12, 2025

View reviewed changes

double bug fix

bfcb7a5

phbnf merged commit 3b77265 into transparency-dev:main May 13, 2025
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deduplication --> Antispam #293

Deduplication --> Antispam #293

phbnf commented May 8, 2025 •

edited

Loading

roger2hk left a comment

roger2hk May 8, 2025

roger2hk May 8, 2025

roger2hk May 8, 2025

roger2hk May 8, 2025

roger2hk May 8, 2025

roger2hk May 8, 2025

phbnf May 8, 2025

roger2hk May 8, 2025

roger2hk May 8, 2025

roger2hk May 8, 2025

roger2hk May 8, 2025

roger2hk May 8, 2025

roger2hk May 8, 2025

AlCutter May 9, 2025

AlCutter May 9, 2025

AlCutter May 9, 2025

AlCutter May 9, 2025

AlCutter May 9, 2025

AlCutter May 12, 2025

phbnf May 12, 2025

AlCutter May 12, 2025

AlCutter May 12, 2025

phbnf May 12, 2025

AlCutter May 12, 2025

phbnf May 12, 2025

	connect the RDS instance to your VM using the instrunctions bellow, and run
	connect the RDS instance to your VM using the instructions below, and run

	# github actions are applying the terraform.
	# GitHub actions are applying the terraform.

	inMemoryAntispamCacheSize = flag.Uint("inmemory_antispam_cache_size", 2<<10, "Maximum number of entries to keep in the in-memory antispam cache.")
	inmemoryAntispamCacheSize = flag.Uint("inmemory_antispam_cache_size", 2<<10, "Maximum number of entries to keep in the in-memory antispam cache.")

	klog.Exitf("Failed to create new GCP antispam storage: %v", err)
	klog.Exitf("Failed to create new AWS antispam storage: %v", err)

	// Empty passord isn't an option with AuroraDB MySQL.
	// Empty password isn't an option with AuroraDB MySQL.

	--antispam_db_name=antispam_db
	--antispam_db_name=antispam_db \

	func antispamMysqlConfig() *mysql.Config {
	func antispamMySQLConfig() *mysql.Config {

Deduplication --> Antispam #293

Deduplication --> Antispam #293

Conversation

phbnf commented May 8, 2025 • edited Loading

roger2hk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phbnf commented May 8, 2025 •

edited

Loading