Add Update structure for appended files and deleted files #235

sezruby · 2020-10-29T10:21:40Z

What is the context for this pull request?

Tracking Issue: Revisit index metadata for mutable dataset #198
Parent Issue: n/a.
Dependencies: Merge RefreshAppendAction and RefreshDeleteAction #232

This change is also required for #169, refresh quick mode.
Fixes #169

What changes were proposed in this pull request?

For #198, we need to keep the latest signature value leveraging appended and deleted files.
This PR defines Update class for newly updated data.

case class Update(
  appendedFiles: Option[Content] = None,
  deletedFiles: Option[Content] = None)

The latest signature value will be replaced with outdated one in RefreshQuickAction.
You can check the use case in #238

Does this PR introduce any user-facing change?

Yes, this is a breakage change, metadata change, so need to rebuild their index.

How was this patch tested?

src/main/scala/com/microsoft/hyperspace/actions/CreateActionBase.scala

src/main/scala/com/microsoft/hyperspace/actions/RefreshActionBase.scala

sezruby · 2020-10-29T10:42:29Z

src/main/scala/com/microsoft/hyperspace/index/IndexLogEntry.scala

 }
 object LogicalPlanFingerprint {
-  case class Properties(signatures: Seq[Signature])
+  case class Properties(signatures: Seq[Signature], planSignature: String)


@imback82 @pirz @apoorvedave1
I wonder this is good approach or no. We need to keep planSignature separately for signature calculation at refreshing - though only logical relation plan is allowed for now.

Hmm, this seems a bit weird. :) Can you point me where this is being used? I couldn't find.

One way to get around (depending on the use case), we can get rid of IndexSignatureProvider and just use two separate signatures: FileBasedSignatureProvider and PlanSignatureProvider. And we can have a utils class that combines the signature values on the rule path.

@imback82 Outdated but for use case: https://github.com/sezruby/hyperspace/blob/metaupdate/src/main/scala/com/microsoft/hyperspace/actions/RefreshAppendAction.scala#L100
https://github.com/sezruby/hyperspace/blob/metaupdate/src/main/scala/com/microsoft/hyperspace/actions/CreateActionBase.scala#L70

~~signatures defined as Seq[Signature] for multiple relations.~~ So do you suggest Seq[Seq[Signature]]?

This is for quick refresh, so no use case for now. Can I create RefreshQuickAction.scala for this w/o exposing API?

signatures defined as Seq[Signature] for multiple relations. So do you suggest Seq[Seq[Signature]]?

Does it make sense to move fingerprint: LogicalPlanFingerprint from SparkPlan to Relation so that the fingerprint always goes with the corresponding relation. (And we can break up IndexSignatureProvider). WDYT?

@apoorvedave1 / @pirz Any suggestions?

Refresh Quick mode

collect appendedFiles, deletedFiles, latestSignature

latestSignature can be calculated by planSignature value + updated file based signature value

current behavior at refresh (full and incremental):

use reconstructed df to get planSignature

Seems we could use the reconstructed df in the same way of the other refresh modes, instead of storing planSignature value separately.
Do you think it is okay to keep the current structure for signature values in IndexLogEntry?

I updated this PR w/o planSignature change and added simple RefreshQuickAction impl for use case.

@imback82 @priz @apoorvedave1 Could you give any quick comment on this?

BTW how about adding stats: Map[String, String] in CoveringIndex.properties that @imback82 mentioned in the previous discussion for hasLineage config? it's just to avoid breaking changes in the next release expecting more statistics / usability changes.

Thanks @sezruby for RefreshQuickAction. The change looks good to me. I left a comment whether we can use LogicalPlanFingerprint. If it works out, could you separate out RefreshQuickAction to a separate PR to better track?

BTW how about adding stats: Map[String, String] in CoveringIndex.properties that @imback82 mentioned in the previous discussion for hasLineage config? it's just to avoid breaking changes in the next release expecting more statistics / usability changes.

There may be an option to update ObjectMapper to allow adding new fields while maintaining compatibility? I am also fine with adding one now. Would this be a "stat" or more like "properties"?

There may be an option to update ObjectMapper to allow adding new fields while maintaining compatibility? I am also fine with adding one now.

Seems no problem to add new fields later. Let's handle it later and wrap up this release 👍

Would this be a "stat" or more like "properties"?

I suggested stats as it will be added under CoveringIndex.Properties. How about extra or aux ..?

imback82 · 2020-11-01T02:38:15Z

src/main/scala/com/microsoft/hyperspace/index/IndexLogEntry.scala

 }

+/**
+ * Updated source data since lst time derived dataset was updated. Note that index data does not


Suggested change

* Updated source data since lst time derived dataset was updated. Note that index data does not

* Updated source data since last time derived dataset was updated. Note that index data does not

Actually, Update is used under Hdfs which is a generic representation of file system. We can make a generic comment here and move this comment to Relation to explain data: Hdfs.

Updated the comment. Thanks!

src/main/scala/com/microsoft/hyperspace/index/IndexLogEntry.scala

imback82 · 2020-11-01T02:58:52Z

src/main/scala/com/microsoft/hyperspace/index/IndexLogEntry.scala

+  // FileInfo's 'name' contains the full path to the file.
+  def deletedFiles: Set[FileInfo] = {
+    sourceUpdate.flatMap(_.deletedFiles).map(_.fileInfos).getOrElse(Set())
  }


Can the following be updated or not (can be done as a follow up PR)?

hyperspace/src/main/scala/com/microsoft/hyperspace/index/rules/RuleUtils.scala

Lines 108 to 115 in 86174a9

// index.deletedFiles and index.appendedFiles should be non-empty until Hybrid Scan

// handles the lists properly. Otherwise, as the source file list of each index entry

// (entry.allSourceFileInfo) also contains the appended and deleted files, we cannot

// get the actual appended files and deleted files correctly.

indexes.filter(

index =>

index.created && index.deletedFiles.isEmpty && index.appendedFiles.isEmpty &&

isHybridScanCandidate(index, filesByRelations.flatten))

It's required until

RefreshQuickAction & refresh mode api

getCandidateIndex fix

compare with the latest signature if exists

~~if so, tag hybrid scan required = true~~

transformPlanToUseIndex & hybrid scan fix

use appendedFiles and deletedFiles if exist

src/main/scala/com/microsoft/hyperspace/index/IndexLogEntry.scala

imback82 · 2020-11-01T03:36:50Z

src/main/scala/com/microsoft/hyperspace/index/IndexLogEntry.scala

+ * @param latestSignatures Latest signatures including appendedFiles and deletedFiles.
+ */
+case class Update(
+  latestSignatures: Seq[Signature],


I thought about this field, and can we make this as fingerprint: LogicalPlanFingerprint? This goes against my previous comment about making this Update generic. Let me explain.

Whenever you see "kind" and "properties", that structure is supposed to be a "plug-in" point. For example, the "Source" has "SparkPlan", but it can be replaced with different sources "SqlDW", etc. It's easier to see if you look at the JSON representation. We never got a chance to work on the parser to support these scenarios and resorted to the case class to make reading/writing JSON easy for the time being.

Back to Update, I think the spec owner can decide what to do with the fingerprint for Update. In our case, it will be a LogicalPlanFingerprint. This also aligns with Content having a fingerprint. Does this make sense? We can update the comment as follows:

/** * Captures any HDFS updates. * * @note 'fingerprint' shouldn't be tied to [[LogicalPlanFingerprint]], but it's a free-form * class, meaning it has "kind" and "properties" so that different classes can be plugged in. * Thus, 'fingerprint' is still a generic field in terms of the metadata log specification. * * @param fingerprint Fingerprint of the update. * @param appendedFiles Appended files. * @param deletedFiles Deleted files. */ case class Update( fingerprint: LogicalPlanFingerprint, appendedFiles: Option[Content] = None, deletedFiles: Option[Content] = None)

That makes sense. Thanks for the suggestion :)

imback82 · 2020-11-02T03:25:48Z

src/main/scala/com/microsoft/hyperspace/index/IndexLogEntry.scala

+ * IndexLogEntry-specific Relation that represents the source relation.
+ *
+ * @param rootPaths List of root paths for the source relation.
+ * @param data Source data since last time derived dataset was updated.


This seems to be mentioning the Update field?
How about

@param data Source data for the relation. Hdfs.properties.content captures source data which derived dataset was created from. Hdfs.properties.update captures any updates since the derived dataset was created. Note that Hdfs.properties.update.fingerprint is a fingerprint created from the latest source data.

@sezruby Writing this comment, Update.fingerprint still feels out of place. :(

What if we update SparkPlan.Properties.fingerprint to always point to the latest fingerprint (instead of having fingerprint inside Update)?

imback82

LGTM, thanks @sezruby! Could you update the PR description?

sezruby changed the title ~~Add Update structure for appended and deleted files and latest signature.~~ Add Update structure for appended and deleted files and its signature. Oct 29, 2020

sezruby changed the title ~~Add Update structure for appended and deleted files and its signature.~~ Add Update structure for appended and deleted files and its signature Oct 29, 2020

sezruby commented Oct 29, 2020

View reviewed changes

src/main/scala/com/microsoft/hyperspace/actions/CreateActionBase.scala Show resolved Hide resolved

src/main/scala/com/microsoft/hyperspace/actions/RefreshActionBase.scala Show resolved Hide resolved

sezruby added the breaking changes label Oct 29, 2020

sezruby added this to the October 2020 milestone Oct 29, 2020

sezruby self-assigned this Oct 29, 2020

sezruby requested review from apoorvedave1, imback82 and pirz October 29, 2020 10:33

sezruby commented Oct 29, 2020

View reviewed changes

sezruby changed the title ~~Add Update structure for appended and deleted files and its signature~~ Add Update structure for appended files and deleted files Oct 29, 2020

sezruby force-pushed the metaupdate2 branch from c723660 to 47ca255 Compare October 30, 2020 02:12

Add Update structure in IndexLogEntry

8cefec8

sezruby force-pushed the metaupdate2 branch from 47ca255 to 8cefec8 Compare October 30, 2020 02:15

imback82 reviewed Nov 1, 2020

View reviewed changes

sezruby added 5 commits November 1, 2020 20:28

review commit

6e25dc4

Remove RefreshQuickAction + fix test

40d6349

comment update

dc8a4a1

Merge remote-tracking branch 'upstream/master' into metaupdate2

53d7029

indent

a94a8df

sezruby mentioned this pull request Nov 2, 2020

Support refresh quick mode by using Hybrid Scan #238

Merged

imback82 reviewed Nov 2, 2020

View reviewed changes

Remove latest fingerprint

5a83b8d

imback82 approved these changes Nov 2, 2020

View reviewed changes

imback82 merged commit 048d875 into microsoft:master Nov 2, 2020

sezruby mentioned this pull request Nov 2, 2020

Revisit index metadata for mutable dataset #198

Closed

sezruby deleted the metaupdate2 branch November 24, 2020 02:47

	* Updated source data since lst time derived dataset was updated. Note that index data does not
	* Updated source data since last time derived dataset was updated. Note that index data does not

	// index.deletedFiles and index.appendedFiles should be non-empty until Hybrid Scan
	// handles the lists properly. Otherwise, as the source file list of each index entry
	// (entry.allSourceFileInfo) also contains the appended and deleted files, we cannot
	// get the actual appended files and deleted files correctly.
	indexes.filter(
	index =>
	index.created && index.deletedFiles.isEmpty && index.appendedFiles.isEmpty &&
	isHybridScanCandidate(index, filesByRelations.flatten))

Add Update structure for appended files and deleted files #235

Add Update structure for appended files and deleted files #235

Uh oh!

Conversation

sezruby commented Oct 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the context for this pull request?

What changes were proposed in this pull request?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sezruby Oct 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sezruby Oct 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sezruby Oct 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

imback82 Nov 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sezruby Nov 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imback82 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sezruby commented Oct 29, 2020 •

edited

Loading

sezruby Oct 30, 2020 •

edited

Loading

sezruby Oct 30, 2020 •

edited

Loading

sezruby Oct 31, 2020 •

edited

Loading

imback82 Nov 1, 2020 •

edited

Loading

sezruby Nov 1, 2020 •

edited

Loading