Skip to content

/ sync

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compaction API #350

Merged
merged 5 commits into from Oct 28, 2019
+257 −32
Merged

Compaction API #350

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5fe3f99
Compact records based on timestamp and keep the latest one
darkdh Oct 16, 2019
df40ca1
Add test for compactCategory
darkdh Oct 18, 2019
b95ffb2
Export COMPACT_SYNC_CATEGORY
darkdh Oct 18, 2019
6e945b2
Address review feedbacks
darkdh Oct 21, 2019
78e8d89
Do compaction in batch with 15 seconds timeout, also support cross batch
darkdh Oct 25, 2019
File filter...
Filter file types
Clear filters
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Prev Next
Loading status checks…

Address review feedbacks

  • Loading branch information
@darkdh
darkdh committed Oct 21, 2019
Verified
This commit was signed with a verified signature.
darkdh Anthony Tseng
GPG key ID: E14094A20D3AAEEC Learn about signing commits
commit 6e945b2897180de7626ab171c0e81bb13e5a788c
4 client/requestUtil.js
View file
Open in desktop
@@ -448,17 +448,21 @@ RequestUtil.prototype.compactCategory = function (category) {
this.withRetry(() => {
return s3Helper.listObjects(this.s3, options, false)
}).then((s3Objects) => {
// This is used to keep the most recent records for each object id
let latestRecords = {}

This comment has been minimized.

Sign in to view
Copy link
@bridiver

bridiver Oct 21, 2019

Contributor

this is going to keep the most recent record in each batch, which is fine, but I think we should add a comment to make that clear
let s3ObjectsToDelete = []
const recordObjects = this.s3ObjectsToRecords(s3Objects.contents)
recordObjects.forEach((recordObject) => {
const record = recordObject.record
This conversation was marked as resolved by AlexeyBarabash

This comment has been minimized.

Sign in to view
Copy link
@AlexeyBarabash

AlexeyBarabash Oct 21, 2019

Contributor

I would ask to make additional logging of the records being purged at compactCategory: action, objectId and timestamp to test and diagnose it easier.
if (latestRecords[record.objectId]) {
console.log('compaction deletes')
if (record.syncTimestamp > latestRecords[record.objectId].record.syncTimestamp) {
s3ObjectsToDelete = s3ObjectsToDelete.concat(latestRecords[record.objectId].objects)
console.log(latestRecords[record.objectId].record)
This conversation was marked as resolved by AlexeyBarabash

This comment has been minimized.

Sign in to view
Copy link
@AlexeyBarabash

AlexeyBarabash Oct 22, 2019

Contributor

@darkdh can you use here ${JSON.stringify(latestRecords[record.objectId].record)}?
right now it looks on bash console as

[12644:12644:1022/150032.455150:INFO:CONSOLE(1)] "compaction deletes", source: chrome-extension://nomlkjnggnifocmealianaaiobmebgil/extension/brave-sync/bundles/bundle.js (1)
[12644:12644:1022/150032.455640:INFO:CONSOLE(1)] "[object Object]", source: chrome-extension://nomlkjnggnifocmealianaaiobmebgil/extension/brave-sync/bundles/bundle.js (1)

and in chromium inspector console it requires to expand and the text search cannot find hidden strings?

This comment has been minimized.

Sign in to view
Copy link
@darkdh

darkdh Oct 22, 2019

Author Member

that is intentional for inspector console to see better structure rather than a single line of string. We should always check inspector console which can be seen for debug and non debug build of brave-core. For the search, the entry will be highlighted, all we need to do is expand it.
latestRecords[record.objectId] = recordObject
} else {
s3ObjectsToDelete = s3ObjectsToDelete.concat(recordObject.objects)
console.log(record)
This conversation was marked as resolved by AlexeyBarabash

This comment has been minimized.

Sign in to view
Copy link
@AlexeyBarabash

AlexeyBarabash Oct 22, 2019

Contributor

and use here also JSON.stringify(record)?
}
} else {
latestRecords[record.objectId] = recordObject
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.